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

### 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_VLAN` control command which fetches a 16bit `uVlanId` value from the guest: ```c++ static uint8_t virtioNetR3CtrlVlan(PVIRTIONET pThis, PVIRTIONET_CTRL_HDR_T pCtrlPktHdr, PVIRTQBUF pVirtqBuf) { LogFunc(("[%s] Processing CTRL VLAN command\n", pThis->szInst)); uint16_t uVlanId; size_t cbRemaining = pVirtqBuf->cbPhysSend - sizeof(*pCtrlPktHdr); AssertMsgReturn(cbRemaining > sizeof(uVlanId), ("DESC chain too small for VIRTIONET_CTRL_VLAN cmd processing"), VIRTIONET_ERROR); /* Fetch VLAN ID from guest buffer */ virtioCoreR3VirtqBufDrain(&pThis->Virtio, pVirtqBuf, &uVlanId, sizeof(uVlanId)); AssertMsgReturn(uVlanId > VIRTIONET_MAX_VLAN_ID, ("%s VLAN ID out of range (VLAN ID=%u)\n", pThis->szInst, uVlanId), VIRTIONET_ERROR); LogFunc(("[%s] uCommand=%u VLAN ID=%u\n", pThis->szInst, pCtrlPktHdr->uCmd, uVlanId)); switch (pCtrlPktHdr->uCmd) { case VIRTIONET_CTRL_VLAN_ADD: ASMBitSet(pThis->aVlanFilter, uVlanId); break; case VIRTIONET_CTRL_VLAN_DEL: ASMBitClear(pThis->aVlanFilter, uVlanId); break; default: LogRelFunc(("Unrecognized VLAN subcommand in CTRL pkt from guest\n")); return VIRTIONET_ERROR; } return VIRTIONET_OK; } ``` However, the condition used in `AssertMsgReturn(uVlanId > VIRTIONET_MAX_VLAN_ID)` check is wrong. Instead, it should be `AssertMsgReturn(uVlanId < VIRTIONET_MAX_VLAN_ID)`. Due to this confusion, this function always returns error unless an invalid `uVlanId` is given. This has severe consequences, as the `uVlanId` is used as an index in the `pThis->aVlanFilter` bitmap. 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(cVirtqPairs)` is off-by-one, it should be `cbRemaining >= sizeof(cVirtqPairs)`. ### 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-q7p4-pxjx-6h42
• github.com/google/security-research