[Dnsmasq-discuss] Hardenining dnsmasq memory allocator against overflows

[Loganaden Velvindron]

Dear dnsmasq developers,

OpenBSD introduced a new API known as reallocarray():

If malloc() must be used with multiplication, be sure to test for overflow:

size_t num, size;
...

/* Check for size_t overflow */
if (size && num > SIZE_MAX / size)
errc(1, EOVERFLOW, "overflow");

if ((p = malloc(size * num)) == NULL)
err(1, "malloc");

The above test is not sufficient in all cases. For example,
multiplying ints requires a different set of checks:

int num, size;
...

/* Avoid invalid requests */
if (size < 0 || num < 0)
errc(1, EOVERFLOW, "overflow");

/* Check for signed int overflow */
if (size && num > INT_MAX / size)
errc(1, EOVERFLOW, "overflow");

if ((p = malloc(size * num)) == NULL)
err(1, "malloc");

Assuming the implementation checks for integer overflow as OpenBSD
does, it is much easier to use calloc() or reallocarray().


The dangers of multiplication integer overflows was what caused the
one of the vulnerabilities in OpenSSH:
https://www.owasp.org/index.php/Integer_overflow

DNSmasq is one of the most popular resolvers, which is often
misconfigured to answer requests from the internet:
http://openresolverproject.org/version.bind.report.txt (Taken drom
Dave Taht's talk at RIPE)

I have conducted a analysis of dnsmasq's multiple malloc instances,
and came up with the following diff:

http://elandsys.com/~logan/dnsmasq_reallocarray.diff

Feedback welcomed
//Logan
C-x-C-c