CVE-2017-15118: QEMU NBD Server Stack Buffer Overflow

Eric Blake reported this vulnerability to qemu-devel mailing list on 28 November 2017. The vulnerability was part of Network Block Device (NBD) server of QEMU. This server allows users to export QEMU disk images using the NBD protocol. Below you can see the key part for this vulnerability, the name size definition from the NBD specification.

Where this document refers to a string, then unless otherwise stated,
that string is a sequence of UTF-8 code points, which is not NUL
terminated, MUST NOT contain NUL characters, SHOULD be no longer than
256 bytes and MUST be no longer than 4096 bytes. This applies
to export names and error messages (amongst others). 

 
So, the recommended maximum length for the exported image’s name is recommended to be 256 bytes and it should never be more than 4096 bytes. Going back to Eric Blake’s report, he noticed that if you try to export an image with a name that is 3000 bytes long the NBD server of QEMU crashes with a stack-based buffer overflow. Below is the command that Eric Blake used to demonstrate this vulnerability.

qemu-io f raw nbd://localhost:10809/$(printf %3000d 1 | tr ' ' a)

 
The vulnerable code is in nbd/nbd-server.c file and specifically in nbd_negotiate_handle_info() function. Below you can see a code snippet of this function before it was patched.

/* Handle NBD_OPT_INFO and NBD_OPT_GO.
 * Return -errno on error, 0 if ready for next option, and 1 to move
 * into transmission phase.  */
static int nbd_negotiate_handle_info(NBDClient *client, uint32_t length,
                                     uint32_t opt, uint16_t myflags,
                                     Error **errp)
{
    int rc;
    char name[NBD_MAX_NAME_SIZE + 1];
   ...
    if (length < sizeof(namelen) + sizeof(requests)) {
        msg = "overall request too short";
   ...
    if (namelen > length - sizeof(requests) || (length - namelen) % 2) {
        msg = "name length is incorrect";
   ...
    if (nbd_read(client->ioc, name, namelen, errp) < 0) {
   ...
}

As you can see, “name” has size of “NBD_MAX_NAME_SIZE” plus one. The code checks for short length as well as invalid length, but never checks if “namelen” is actually larger than the size of the “name”. This means that using Eric Blake’s example, nbd_read() will end up trying to copy 3000+ bytes from “client->ioc” to “name” buffer. However, if we check include/block/nbd.h header file we’ll see that “NBD_MAX_NAME_SIZE” is just 256.

/* Maximum size of an export name. The NBD spec requires 256 and
 * suggests that servers support up to 4096, but we stick to only the
 * required size so that we can stack-allocate the names, and because
 * going larger would require an audit of more code to make sure we
 * aren't overflowing some other buffer. */
#define NBD_MAX_NAME_SIZE 256

The patch to fix this vulnerability was to add the missing check from the call to ndb_read() in nbd/server.c code. You can see the diff of this patch below.

     }
+    if (namelen >= sizeof(name)) {
+        msg = "name too long for qemu";
+        goto invalid;
+    }
     if (nbd_read(client->ioc, name, namelen, errp) < 0) {