Oracle VM VirtualBox: Intra-Object Out-Of-Bounds Write in virtioNetR3CtrlMultiQueue

### Summary A guest inside a VirtualBox VM using the virtio-net network adapter can trigger an intra-object out-of-bounds write in `src/VBox/Devices/Network/DevVirtioNet.cpp` to cause a denial-of-service or escape the hypervisor and compromise the host. ### Severity High - An attacker with high privileges in the guest can cause a denial-of-service or escape the hypervisor and compromise the host. ### Proof of Concept The following function handles a `VIRTIONET_CTRL_MQ` control command which fetches a 16bit `cVirtqPairs` value from the guest: ```c++ static uint8_t virtioNetR3CtrlMultiQueue(PVIRTIONET pThis, PVIRTIONETCC pThisCC, PPDMDEVINS pDevIns, PVIRTIONET_CTRL_HDR_T pCtrlPktHdr, PVIRTQBUF pVirtqBuf) { LogFunc(("[%s] Processing CTRL MQ command\n", pThis->szInst)); uint16_t cVirtqPairs; switch(pCtrlPktHdr->uCmd) { case VIRTIONET_CTRL_MQ_VQ_PAIRS_SET: { size_t cbRemaining = pVirtqBuf->cbPhysSend - sizeof(*pCtrlPktHdr); AssertMsgReturn(cbRemaining > sizeof(cVirtqPairs), ("DESC chain too small for VIRTIONET_CTRL_MQ cmd processing"), VIRTIONET_ERROR); /* Fetch number of virtq pairs from guest buffer */ virtioCoreR3VirtqBufDrain(&pThis->Virtio, pVirtqBuf, &cVirtqPairs, sizeof(cVirtqPairs)); AssertMsgReturn(cVirtqPairs > VIRTIONET_MAX_QPAIRS, ("[%s] Guest CTRL MQ virtq pair count out of range [%d])\n", pThis->szInst, cVirtqPairs), VIRTIONET_ERROR); LogFunc(("[%s] Guest specifies %d VQ pairs in use\n", pThis->szInst, cVirtqPairs)); pThis->cVirtqPairs = cVirtqPairs; break; } default: LogRelFunc(("Unrecognized multiqueue subcommand in CTRL pkt from guest\n")); return VIRTIONET_ERROR; } // ... if (pThis->cVirtqPairs > pThis->cInitializedVirtqPairs) { virtioNetR3SetVirtqNames(pThis, virtioCoreIsLegacyMode(&pThis->Virtio)); int rc = virtioNetR3CreateWorkerThreads(pDevIns, pThis, pThisCC); if (RT_FAILURE(rc)) { LogRelFunc(("Failed to create worker threads\n")); return VIRTIONET_ERROR; } } return VIRTIONET_OK; } ``` However, the condition used in `AssertMsgReturn(cVirtqPairs > VIRTIONET_MAX_QPAIRS)` check is wrong. Instead, it should be `AssertMsgReturn(cVirtqPairs < VIRTIONET_MAX_QPAIRS)`. Due to this confusion, this function always returns error unless an invalid `cVirtqPairs` is given. This has severe consequences, as the `pThis->cVirtqPairs` is used as a a limit for the `pThis->aVirtqs` array in `virtioNetR3SetVirtqNames()`, `virtioNetR3CreateWorkerThreads()` and more: ```c++ DECLINLINE(void) virtioNetR3SetVirtqNames(PVIRTIONET pThis, uint32_t fLegacy) { RTStrCopy(pThis->aVirtqs[CTRLQIDX].szName, VIRTIO_MAX_VIRTQ_NAME_SIZE, fLegacy ? "legacy-ctrlq" : " modern-ctrlq"); for (uint16_t qPairIdx = 0; qPairIdx < pThis->cVirtqPairs; qPairIdx++) { RTStrPrintf(pThis->aVirtqs[RXQIDX(qPairIdx)].szName, VIRTIO_MAX_VIRTQ_NAME_SIZE, "%s-recvq<%d>", fLegacy ? "legacy" : "modern", qPairIdx); RTStrPrintf(pThis->aVirtqs[TXQIDX(qPairIdx)].szName, VIRTIO_MAX_VIRTQ_NAME_SIZE, "%s-xmitq<%d>", fLegacy ? "legacy" : "modern", qPairIdx); } } ``` There are additional bugs in the code: - The function `virtioCoreR3VirtqBufDrain()` called in `virtioNetR3Ctrl()` already decreases `pVirtqBuf->cbPhysSend`, hence the `pVirtqBuf->cbPhysSend - sizeof(*pCtrlPktHdr)` calculation is wrong. It should be `pVirtqBuf->cbPhysSend`. - The check `cbRemaining > sizeof(uVlanId)` is off-by-one, it should be `cbRemaining >= sizeof(uVlanId)`. ### Timeline **Date reported**: 08/15/2023 **Date fixed**: 10/17/2023 **Date disclosed**: 11/16/2023

Severity from

GitHub (reviewed advisory)

• github.com/google/security-research/security/advisories/GHSA-6wqg-7grf-6q38
• github.com/google/security-research