Harden dropbear memory allocator

Matt Johnston matt at ucc.asn.au
Sun Apr 12 20:34:50 AWST 2015


Hi Logan,

Thanks for looking at that - certainly something to be wary
of. I've actually removed netio.c's malloc altogether a
couple of weeks ago though had neglected to push it until
now https://secure.ucc.asn.au/hg/dropbear/rev/cc6116cc0b5c
(my github mirror isn't updated, the machine that does that is
offline, should be back next week).

I think netio.c was OK because *ret_iov_count was limited to
IOV_MAX which is 1024 or less on most platforms.

Cheers,
Matt


On Sun, Apr 12, 2015 at 08:19:00AM +0000, Loganaden Velvindron wrote:
> 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