Harden dropbear memory allocator

Loganaden Velvindron loganaden at gmail.com
Sun Apr 12 16:19:00 AWST 2015


Hi All,

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

dropbear is one of the most popular ssh servers for home routers.

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

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

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


More information about the Dropbear mailing list