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:
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:
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
TheOfficialFloW Finder
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.cppto 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_MQcontrol command which fetches a 16bitcVirtqPairsvalue from the guest:However, the condition used in
AssertMsgReturn(cVirtqPairs > VIRTIONET_MAX_QPAIRS)check is wrong. Instead, it should beAssertMsgReturn(cVirtqPairs < VIRTIONET_MAX_QPAIRS). Due to this confusion, this function always returns error unless an invalidcVirtqPairsis given. This has severe consequences, as thepThis->cVirtqPairsis used as a a limit for thepThis->aVirtqsarray invirtioNetR3SetVirtqNames(),virtioNetR3CreateWorkerThreads()and more:There are additional bugs in the code:
virtioCoreR3VirtqBufDrain()called invirtioNetR3Ctrl()already decreasespVirtqBuf->cbPhysSend, hence thepVirtqBuf->cbPhysSend - sizeof(*pCtrlPktHdr)calculation is wrong. It should bepVirtqBuf->cbPhysSend.cbRemaining > sizeof(uVlanId)is off-by-one, it should becbRemaining >= sizeof(uVlanId).Timeline
Date reported: 08/15/2023
Date fixed: 10/17/2023
Date disclosed: 11/16/2023