[Libguestfs] [PATCH nbdkit 0/4] Multiple valgrind improvements and possible security fix.

[Richard W.M. Jones]

On Mon, Dec 03, 2018 at 01:03:07PM -0600, Eric Blake wrote:
> On 12/2/18 10:39 AM, Richard W.M. Jones wrote:
> >I worked out why valgrind wasn't being applied to nbdkit when run by
> >many of the tests (patches 1-2).  Unfortunately I'm not able to make
> >it actually fail tests when valgrind fails.  Although the situation is
> >marginally improved in that you can now manually examine the *.log
> >files and find valgrind failures that way.  Also adds valgrinding of
> >the Python plugin (patch 3).
> >
> >Along the way I found that when we create a TLS session object we
> >never free it, which is a bit of a problem (although easy to fix -
> >patch 4).
> >
> >I'll need to backport this fix to every stable branch.  It's not clear
> >how exploitable this is -- it's my feeling that you'd need to open
> >millions of TLS sessions which would take forever, and the result
> >would only be a denial of service as nbdkit runs out of memory and
> >crashes.
> 
> Can the leak happen with merely a port probe, or only by someone
> that was able to get past the handshake?  If the former, then it is
> a vehicle for DoS attacks and probably worth a CVE (because the
> person performing the port probe can crash nbdkit from servicing
> real clients, even though the attacker does not own TLS
> credentials);

We allocate the session here:

https://github.com/libguestfs/nbdkit/blob/dae18932153d7249fe74bfcec893115d1f006793/src/crypto.c#L408

Along every error path the function frees the session.

The handshake happens here:

https://github.com/libguestfs/nbdkit/blob/dae18932153d7249fe74bfcec893115d1f006793/src/crypto.c#L494

If it fails we jump to the ‘error:’ label which will free the session.
The leak only happens if we handshake successfully, in which case the
function returns 0 and the session is never freed after that.

> if the latter, it is not a security bug (no escalation
> in privilege by locking yourself out).

It's possible to run a public facing nbdkit server with TLS which does
not check client certificates nor PSKs (in fact, that's the default).
So it's likely a DoS.  My reason for saying it's low priority is that
you'd need to initiate quite a lot of TLS sessions to leak any
significant amount of memory.

According to valgrind each session leaks 13,996 bytes.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org