Sun xVM VirtualBox Guest Additions Memory Consumption (on guest OS)

This bug was recently disclosed by Sun Microsystems and it affects 1.6, 2.0.0, 2.0.2, 2.0.4, 2.0.6, 2.0.8, 2.0.10, 2.1, 2.2, 3.0.0, 3.0.2, 3.0.4, 3.0.6 and 3.0.8 releases of “Guest Additions” software provided by Sun xVM VirtualBox.
Guest Additions is a software available for Windows, Linux and Solaris and it is used in the installed guest operating systems for performance and integration purposes.
So, back to the actual code. As we can read in src/VBox/Additions/common/VBoxGuest/VBoxGuest.cpp of 3.0.8 release of VirtualBox we can find the following…

static int VBoxGuestCommonIOCtl_VMMRequest(PVBOXGUESTDEVEXT pDevExt, PVBOXGUESTSESSION pSession,
                                           VMMDevRequestHeader *pReqHdr, size_t cbData, size_t *pcbDataReturned)
{
    Log(("VBoxGuestCommonIOCtl: VMMREQUEST type %d\n", pReqHdr->requestType));

    /*
     * Validate the header and request size.
     */
    const VMMDevRequestType enmType   = pReqHdr->requestType;
    const uint32_t          cbReq     = pReqHdr->size;
    const uint32_t          cbMinSize = vmmdevGetRequestSize(enmType);
    if (cbReq < cbMinSize)
  ...
    if (cbReq > cbData)
  ...

    /*
     * Make a copy of the request in the physical memory heap so
     * the VBoxGuestLibrary can more easily deal with the request.
     * (This is really a waste of time since the OS or the OS specific
     * code has already buffered or locked the input/output buffer, but
     * it does makes things a bit simpler wrt to phys address.)
     */
    VMMDevRequestHeader *pReqCopy;
    int rc = VbglGRAlloc(&pReqCopy, cbReq, enmType);
    if (RT_FAILURE(rc))
    {
        Log(("VBoxGuestCommonIOCtl: VMMREQUEST: failed to allocate %u (%#x) bytes to cache the request. rc=%d!!\n",
             cbReq, cbReq, rc));
        return rc;
    }
    memcpy(pReqCopy, pReqHdr, cbReq);
 ...
    VbglGRFree(pReqCopy);
    return rc;
}

The aboce IOCTL handler is used by the aforementioned guest addition utilities software to perform a request on the hypervisor VMM. The request’s information are stored in a ‘VMMDevRequestHeader’ object which is passed as a pointer using ‘pReqHdr’ variable.
After initializing some variables with the request type and size, a check takes place to ensure that the minimum default size for that request is larger than the size contained in the request header. After that, a second size check takes place to ensure that the requested data fit in the requested size. Assuming that both conditions are true, VirtualBox will attempt to copy the request in heap memory using VbglGRAlloc() to allocate the required space and memcpy() to copy the actual data of the request. However, the request might still request more memory than it actually needs since it only checks that the space is more than the minimum. Because of this missing check, an attacker can create a malicious call to that IOCTL with a header size large enough to consume a lot of kernel memory and bring the guest OS in an unstable state.
To fix this, the above function was changed like this:

         return VERR_INVALID_PARAMETER;
     }
+    int rc = VbglGRVerify(pReqHdr, cbData);
+    if (RT_FAILURE(rc))
+    {
+        Log(("VBoxGuestCommonIOCtl: VMMREQUEST: invalid header: size %#x, expected >= %#x (hdr); type=%#x; rc %d!!\n",
+             cbData, cbReq, enmType, rc));
+        return rc;
+    }
 
     /*

And this VbglGRVerify() is a newly added routine which resides in src/VBox/Additions/common/VBoxGuestLib/GenericRequest.cpp like this:

DECLVBGL(int) VbglGRVerify (const VMMDevRequestHeader *pReq, size_t cbReq)
{
    if (!pReq || cbReq < sizeof (VMMDevRequestHeader))
    {
        dprintf(("VbglGRVerify: Invalid parameter: pReq = %p, cbReq = %d\n", pReq, cbReq));
        return VERR_INVALID_PARAMETER;
    }

    if (pReq->size > cbReq)
    {
        dprintf(("VbglGRVerify: request size %d > buffer size %d\n", pReq->size, cbReq));
        return VERR_INVALID_PARAMETER;
    }

    /* The request size must correspond to the request type. */
    size_t cbReqExpected = vmmdevGetRequestSize(pReq->requestType);

    if (cbReq < cbReqExpected)
    {
        dprintf(("VbglGRVerify: buffer size %d < expected size %d\n", cbReq, cbReqExpected));
        return VERR_INVALID_PARAMETER;
    }

    if (cbReqExpected == cbReq)
    {
        /* This is most likely a fixed size request, and in this case the request size
         * must be also equal to the expected size.
         */
        if (pReq->size != cbReqExpected)
        {
            dprintf(("VbglGRVerify: request size %d != expected size %d\n", pReq->size, cbReqExpected));
            return VERR_INVALID_PARAMETER;
        }

        return VINF_SUCCESS;
    }

    /* This can be a variable size request. Check the request type and limit the size
     * to VMMDEV_MAX_VMMDEVREQ_SIZE, which is max size supported by the host. 
     */
    if (   pReq->requestType == VMMDevReq_LogString
        || pReq->requestType == VMMDevReq_VideoSetVisibleRegion
        || pReq->requestType == VMMDevReq_SetPointerShape
#ifdef VBOX_WITH_64_BITS_GUESTS
        || pReq->requestType == VMMDevReq_HGCMCall32
        || pReq->requestType == VMMDevReq_HGCMCall64
#else
        || pReq->requestType == VMMDevReq_HGCMCall
#endif /* VBOX_WITH_64_BITS_GUESTS */
        || pReq->requestType == VMMDevReq_ChangeMemBalloon)
    {
        if (cbReq > VMMDEV_MAX_VMMDEVREQ_SIZE)
        {
            dprintf(("VbglGRVerify: VMMDevReq_LogString: buffer size %d too big\n", cbReq));
            return VERR_BUFFER_OVERFLOW; /* @todo is this error code ok? */
        }
    }
    else
    {
        dprintf(("VbglGRVerify: request size %d > buffer size %d\n", pReq->size, cbReq));
        return VERR_IO_BAD_LENGTH; /* @todo is this error code ok? */
    }

    return VINF_SUCCESS;
}

What it does is a series of checks the validate the request header’s contents. Specifically, it checks the following:
– The request is not NULL
– Data have at least the size of a valid request object
– The request’s size is larger than the data to be copied
– The data’s size cannot be less than the request’s expected size
– If the data’s size is equal to the expected one and the request’s size is not different of the expected one, return with success
– if the request type is VMMDevReq_LogString, VMMDevReq_VideoSetVisibleRegion, VMMDevReq_SetPointerShape as well as some 64-bit OS specific ones which are: VMMDevReq_HGCMCall32, VMMDevReq_HGCMCall64 and VMMDevReq_HGCMCall, check that the data aren’t larger than VMMDEV_MAX_VMMDEVREQ_SIZE constant.
Using this, the IOCTL handler will catch any invalid request that could lead in memory consumption and eventually a DoS situation in the guest OS when Guest Additions software is installed.
Similar checks were also added in VBoxGuestDeviceControl() function located in src/VBox/Additions/WINNT/VBoxGuest/VBoxGuest.cpp like this:

            /* make sure the buffers suit the request */
             CHECK_SIZE(vmmdevGetRequestSize(requestHeader->requestType));
 
+            int rc = VbglGRVerify(requestHeader, requestHeader->size);
+            if (RT_FAILURE(rc))
+            {
+                dprintf(("VBoxGuest::VBoxGuestDeviceControl: VMMREQUEST: invalid header: size %#x, expected >= %#x (hdr); type=%#x; rc %d!!\n",
+                     requestHeader->size, vmmdevGetRequestSize(requestHeader->requestType), requestHeader->requestType, rc));
+                Status = STATUS_INVALID_PARAMETER;
+                break;
+            }
+
             /* just perform the request */
             VMMDevRequestHeader *req = NULL;
 
-            int rc = VbglGRAlloc((VMMDevRequestHeader **)&req, requestHeader->size, requestHeader->requestType);
+            rc = VbglGRAlloc((VMMDevRequestHeader **)&req, requestHeader->size, requestHeader->requestType);
 
             if (RT_SUCCESS(rc))

As well as vboxadd_ioctl() located in src/VBox/Additions/linux/module/vboxmod.c…

         if (0 == rc)
         {
+            rc = VbglGRVerify(&reqHeader, cbRequestSize);
+            if (RT_FAILURE(rc))
+            {
+                Log(("VBOXGUEST_IOCTL_VMMREQUEST: invalid request header: size %d min: %d type: %d rc: %d\n",
+                     cbRequestSize,
+                     cbVanillaRequestSize,
+                     reqHeader.requestType,
+                     rc));
+                rc = -EINVAL;
+            }
+            else
+                rc = 0;
+        }
+        if (0 == rc)
+        {
             /* request storage for the full request */

And at last, the official API definition for the newly added function can be found in include/VBox/VBoxGuestLib.h.

/**
 * Verify the generic request header.
 *
 * @param pReq     pointer the request header structure.
 * @param cbReq    size of the request memory block. It should be equal to the request size
 *                 for fixed size requests. It can be greater than the request size for
 *                 variable size requests.
 *
 * @return VBox status code.
 */
DECLVBGL(int) VbglGRVerify (const VMMDevRequestHeader *pReq, size_t cbReq);