Re: libvnc and tightvnc vulnerabilities

[Solar Designer <solar () openwall com>]

On Mon, Dec 10, 2018 at 07:57:21PM +0100, Solar Designer wrote:
> https://github.com/LibVNC/libvncserver/issues/247
>
> Upstream's fix appears to be to add casts to (uint64_t) before adding 1
> in those many malloc() calls.  On platforms with larger than 32-bit
> size_t, this should be sufficient against integer overflows since the
> sizes are read from 32-bit protocol fields, but it isn't sufficient to
> prevent maliciously large memory allocation on the client by a rogue
> server.  On a platform with 32-bit size_t, this isn't even sufficient to
> prevent the integer overflows.  If I haven't missed anything, it'd be
> great if you open a new issue suggesting introduction of safety limits
> prior to those malloc() lines.

> [...] per the commits referenced in issue #247 above, there are many more
> instances of the "malloc(... + 1)" pattern, which were patched similarly
> incompletely.

I've just created this issue:

SECURITY: malloc((uint64_t)length + 1) is unsafe, especially on 32-bit systems
https://github.com/LibVNC/libvncserver/issues/273

Alexander