[Dnsmasq-discuss] NULL dereference in cache_insert

[Simon Kelley]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Thanks for that.

The problem occurs when there's an A (or AAAA) record defined locally
in a hosts file, and then some reply from upstream includes an empty
record for the same name.

The code is supposed to check for a clash between local and upstream,
ie different addresses, and if there is one, it handles that
differently to the case where upstream gives the _same_ address as the
local definition. The case that upstream returns "no data" wasn't
handled properly, it should be part of the "clash" code-path, since
hosts files can't define a name to be empty.

Your fix is exactly the right thing to do, and I've committed it to
the git repo.

Cheers,

Simon.


On 14/11/15 09:57, Török Edwin wrote:
> Hi,
> 
> dnsmasq 2.73 and 2.75 on OpenWrt CHAOS CALMER (15.05, r46767)
> crashes with a NULL dereference when certain domain names are
> resolved that are also overridden using addn-hosts:
> 
> #0  0x00405612 in cache_insert (name=name at entry=0x42f008
> "bbc.112.2o7.net", addr=addr at entry=0x0, now=now at entry=1447493529,
> ttl=45, flags=296) at cache.c:490 else if ((flags & F_IPV6) &&
> (new->flags & F_IPV6) && 
> IN6_ARE_ADDR_EQUAL(&new->addr.addr.addr.addr6, &addr->addr.addr6))
> 
> #1  0x0040683e in extract_addresses (header=header at entry=0x42fb30,
> qlen=qlen at entry=122, name=<optimized out>,
> now=now at entry=1447493529, ipsets=ipsets at entry=0x0, is_sign=0,
> check_rebind=check_rebind at entry=1,
> no_cache_dnssec=no_cache_dnssec at entry=0, secure=secure at entry=0,
> doctored=doctored at entry=0x7fff6a60) at rfc1035.c:1132
> 
> newc = cache_insert(name, NULL, now, ttl ? ttl : cttl, F_FORWARD |
> F_NEG | flags | secflag);
> 
> #2  0x0040ca1e in process_reply (header=header at entry=0x42fb30,
> now=now at entry=1447493529, server=server at entry=0x437e70,
> n=n at entry=122, check_rebind=1, no_cache=0, added_pheader=0,
> check_subnet=0, query_source=query_source at entry=0x4383e0,
> do_bit=<optimized out>, ad_reqd=<optimized out>, bogusanswer=0,
> cache_secure=0) at forward.c:644 #3  0x0040d864 in reply_query
> (fd=<optimized out>, family=<optimized out>,
> now=now at entry=1447493529) at forward.c:1095 #4  0x0040f2c0 in
> check_dns_listeners (now=now at entry=1447493529) at dnsmasq.c:1510 #5
> 0x00403a3c in main (argc=<optimized out>, argv=<optimized out>) at
> dnsmasq.c:1004
> 
> As a temporary workaround to stop it from crashing I have added a
> NULL check, but I'm not sure what the intended behaviour would be
> when addr is NULL:
> 
> ---
> ./build_dir/target-mips_34kc_uClibc-0.9.33.2/dnsmasq-dhcpv6/dnsmasq-2.
75/src/cache.c
> 2015-07-30 22:59:07.000000000 +0300 +++
> ./build_dir/target-mips_34kc_uClibc-0.9.33.2/dnsmasq-nodhcpv6/dnsmasq-
2.75/src/cache.c
> 2015-11-14 11:41:52.655551879 +0200 @@ -481,7 +481,7 @@ existing
> record is for an A or AAAA and the record we're trying to insert is
> the same, just drop the insert, but don't error the whole process.
> */ -      if ((flags & (F_IPV4 | F_IPV6)) && (flags & F_FORWARD)) +
> if ((flags & (F_IPV4 | F_IPV6)) && (flags & F_FORWARD) && addr)
> 
> $ grep bbc.112 /etc/block.hosts 0.0.0.0 bbc.112.2o7.net ::
> bbc.112.2o7.net $ cat /var/etc/dnsmasq.conf # auto-generated config
> file from /etc/config/dhcp conf-file=/etc/dnsmasq.conf 
> dhcp-authoritative domain-needed localise-queries read-ethers 
> bogus-priv expand-hosts local-service domain=lan server=/lan/ 
> addn-hosts=/etc/block.hosts dhcp-leasefile=/tmp/dhcp.leases 
> resolv-file=/tmp/resolv.conf.auto stop-dns-rebind 
> rebind-localhost-ok rebind-domain-ok=skylable.com 
> dhcp-broadcast=tag:needs-broadcast addn-hosts=/tmp/hosts 
> conf-dir=/tmp/dnsmasq.d
> 
> dhcp-host=e0:3f:49:a0:6e:d4,192.168.1.2,debian 
> dhcp-host=64:51:06:22:ad:dd,192.168.1.129,hp
> 
> 
> 
> dhcp-range=lan,192.168.1.100,192.168.1.249,255.255.255.0,12h 
> no-dhcp-interface=pppoe-wan
> 
> 
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBCAAGBQJWR3XnAAoJEBXN2mrhkTWitBsP/30LEuBBwoG8wwIhhq6Ga4a3
NNd6DWs2a6/pNna/tiUUjxvoq2Qsou/auz0fwT42KjCO1muXGdNB6DJ+R3iNtyTQ
CTWMpZqKlyC++DrjHUsXzHX7Rqm2JRrlHlVYK5osa+05t4Nlr2vXuoKa3g3E+teW
u6OevzUsyr6wfS72TOyppoCmuoYA1qX2kIqN/Q2r9VwzsE9WYk7+nuivOUqcsF8q
W+VkPGhg/ifBIh65ju5w6m8eu1rSOMjEEy0xwxbBeQTlVU2UA2fL4nalpK4tqf++
XygeY4gKldo6Ot9L5vQDXnWHes+9VIYuk9L1/UDOYadqdTBZNoPKIWXnwMqTNc85
rnRR/n8NAmwlumF6IKNEaygVdWpD9h1kf5Yr3qu9wweeHeof3VKiGPS5SoEJ5Qzd
n0Q6iVijwoZF6i3LuyygHUIaegxqwGn+RPtKVUVoMDDZdDIU4LqNiU56QJwKYWAG
5vN3CRFDBlggdEvHgl7UTKwVjVgiWZXSUIUfx9CiGzXR+s7960z7h4ZHWH3/Qd/j
LfBArGM3syDYGv1Z2kOWBr+7PlOYoaRnegX1ZLKD+MrsHqYCXsiM6x5nYbIIjyqm
I8KT6SuS+k2ypxgWOjJLbPXzHd9xW5MTFkXVeDAXEXFwx7KSLqgI4AbZtTR+acas
J1jrBfHRlPcLY+ri5ZUD
=ISXa
-----END PGP SIGNATURE-----