Skip to content

Commit f03a69d

Browse files
VP-at-oracleVP-at-oracle
committed
Devices/Security: UsbCardReader: report error to the guest for invalid blocks. bugref:11098
svn:sync-xref-src-repo-rev: r174539
1 parent bc9176b commit f03a69d

1 file changed

Lines changed: 161 additions & 108 deletions

File tree

‎src/VBox/Devices/Security/UsbCardReader.cpp‎

Lines changed: 161 additions & 108 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* $Id: UsbCardReader.cpp 114701 2026-07-14 13:09:24Z vitali.pelenjow@oracle.com $ */
1+
/* $Id: UsbCardReader.cpp 114703 2026-07-14 13:23:44Z vitali.pelenjow@oracle.com $ */
22
/** @file
33
* UsbCardReader - Usb Smart Card Reader implementation.
44
*/
@@ -1461,44 +1461,58 @@ static int usbCardReaderICCSetParameters(PUSBCARDREADER pThis,
14611461
/* The CCID claims automatic parameters negotiation and allows to change only bmFindexDindex. */
14621462
if (pHdr->u.PC_to_RDR.u.SetParameters.bProtocolNum == 0)
14631463
{
1464-
VUSBCARDREADERPARMST0 *pT0 = (VUSBCARDREADERPARMST0 *)&pHdr[1];
1464+
if (pCmd->dwLength >= sizeof(VUSBCARDREADERPARMST0))
1465+
{
1466+
VUSBCARDREADERPARMST0 *pT0 = (VUSBCARDREADERPARMST0 *)&pHdr[1];
14651467

1466-
UCRLOG(("T0: bmFindexDindex 0x%02X, bmTCCKST0 0x%02X, bGuardTimeT0 0x%02X,"
1467-
" bWaitingIntegerT0 0x%02X, bClockStop 0x%02X\n",
1468-
pT0->bmFindexDindex, pT0->bmTCCKST0, pT0->bGuardTimeT0,
1469-
pT0->bWaitingIntegerT0, pT0->bClockStop));
1468+
UCRLOG(("T0: bmFindexDindex 0x%02X, bmTCCKST0 0x%02X, bGuardTimeT0 0x%02X,"
1469+
" bWaitingIntegerT0 0x%02X, bClockStop 0x%02X\n",
1470+
pT0->bmFindexDindex, pT0->bmTCCKST0, pT0->bGuardTimeT0,
1471+
pT0->bWaitingIntegerT0, pT0->bClockStop));
14701472

1471-
pSlot->ParmsT0.bmFindexDindex = pT0->bmFindexDindex;
1473+
pSlot->ParmsT0.bmFindexDindex = pT0->bmFindexDindex;
1474+
}
1475+
else
1476+
{
1477+
u8UnsupportedOffset = (uint8_t)RT_UOFFSETOF(VUSBCARDREADERBULKHDR, u.PC_to_RDR.u.SetParameters.bProtocolNum);
1478+
}
14721479
}
14731480
else if (pHdr->u.PC_to_RDR.u.SetParameters.bProtocolNum == 1)
14741481
{
1475-
VUSBCARDREADERPARMST1 *pT1 = (VUSBCARDREADERPARMST1 *)&pHdr[1];
1482+
if (pCmd->dwLength >= sizeof(VUSBCARDREADERPARMST1))
1483+
{
1484+
VUSBCARDREADERPARMST1 *pT1 = (VUSBCARDREADERPARMST1 *)&pHdr[1];
14761485

1477-
UCRLOG(("T1: bmFindexDindex 0x%02X, bmTCCKST1 0x%02X, bGuardTimeT1 0x%02X,"
1478-
" bmWaitingIntegersT1 0x%02X, bClockStop 0x%02X, bIFSC 0x%02X, bNadValue 0x%02X\n",
1479-
pT1->bmFindexDindex, pT1->bmTCCKST1, pT1->bGuardTimeT1,
1480-
pT1->bmWaitingIntegersT1, pT1->bClockStop, pT1->bIFSC, pT1->bNadValue));
1486+
UCRLOG(("T1: bmFindexDindex 0x%02X, bmTCCKST1 0x%02X, bGuardTimeT1 0x%02X,"
1487+
" bmWaitingIntegersT1 0x%02X, bClockStop 0x%02X, bIFSC 0x%02X, bNadValue 0x%02X\n",
1488+
pT1->bmFindexDindex, pT1->bmTCCKST1, pT1->bGuardTimeT1,
1489+
pT1->bmWaitingIntegersT1, pT1->bClockStop, pT1->bIFSC, pT1->bNadValue));
14811490

1482-
/* Check paramaters. */
1483-
if (pT1->bIFSC > 254)
1484-
{
1485-
u8UnsupportedOffset = (uint8_t)(RT_UOFFSETOF(VUSBCARDREADERPARMST1, bmTCCKST1) + sizeof(VUSBCARDREADERBULKHDR));
1486-
}
1491+
/* Check paramaters. */
1492+
if (pT1->bIFSC > 254)
1493+
{
1494+
u8UnsupportedOffset = (uint8_t)(RT_UOFFSETOF(VUSBCARDREADERPARMST1, bmTCCKST1) + sizeof(VUSBCARDREADERBULKHDR));
1495+
}
14871496

1488-
if (u8UnsupportedOffset == 0)
1497+
if (u8UnsupportedOffset == 0)
1498+
{
1499+
/* Change parameters only if there is no error. */
1500+
pSlot->ParmsT1.bmFindexDindex = pT1->bmFindexDindex;
1501+
1502+
#define UPDATEPARM(parm) if (pT1->parm != 0) pSlot->ParmsT1.parm = pT1->parm
1503+
UPDATEPARM(bmFindexDindex);
1504+
UPDATEPARM(bmTCCKST1);
1505+
UPDATEPARM(bGuardTimeT1);
1506+
UPDATEPARM(bmWaitingIntegersT1);
1507+
UPDATEPARM(bClockStop);
1508+
UPDATEPARM(bIFSC);
1509+
UPDATEPARM(bNadValue);
1510+
#undef UPDATEPARM
1511+
}
1512+
}
1513+
else
14891514
{
1490-
/* Change parameters only if there is no error. */
1491-
pSlot->ParmsT1.bmFindexDindex = pT1->bmFindexDindex;
1492-
1493-
#define UPDATEPARM(parm) if (pT1->parm != 0) pSlot->ParmsT1.parm = pT1->parm
1494-
UPDATEPARM(bmFindexDindex);
1495-
UPDATEPARM(bmTCCKST1);
1496-
UPDATEPARM(bGuardTimeT1);
1497-
UPDATEPARM(bmWaitingIntegersT1);
1498-
UPDATEPARM(bClockStop);
1499-
UPDATEPARM(bIFSC);
1500-
UPDATEPARM(bNadValue);
1501-
#undef UPDATEPARM
1515+
u8UnsupportedOffset = (uint8_t)RT_UOFFSETOF(VUSBCARDREADERBULKHDR, u.PC_to_RDR.u.SetParameters.bProtocolNum);
15021516
}
15031517
}
15041518
else
@@ -1634,6 +1648,8 @@ static bool usbCardReaderT1ValidateChkSum(PCARDREADERSLOT pSlot, const uint8_t *
16341648

16351649
uint8_t au8Sum[2];
16361650
uint8_t cbSum = usbCardReaderIsCrc16ChkSum(pSlot) ? 2 : 1;
1651+
if (cbBlock < cbSum)
1652+
return false;
16371653

16381654
int rc = usbCardReaderT1ChkSum(pSlot, au8Sum, pbBlock, cbBlock - cbSum);
16391655

@@ -1661,14 +1677,13 @@ static int usbCardReaderT1CreateBlock(PCARDREADERSLOT pSlot,
16611677

16621678
int rc = VINF_SUCCESS;
16631679

1664-
uint32_t cbChkSum = usbCardReaderIsCrc16ChkSum(pSlot) ? 2 : 1;
1680+
uint32_t const cbChkSum = usbCardReaderIsCrc16ChkSum(pSlot) ? 2 : 1;
16651681

1666-
PT1BLKHEADER pT1Blk = NULL;
1667-
uint32_t cbT1Blk = cbT1BodyBlock
1668-
+ sizeof(T1BLKHEADER)
1669-
+ cbChkSum;
1682+
uint32_t const cbT1Blk = sizeof(T1BLKHEADER)
1683+
+ cbT1BodyBlock
1684+
+ cbChkSum;
16701685

1671-
pT1Blk = (PT1BLKHEADER)RTMemAllocZ(cbT1Blk);
1686+
PT1BLKHEADER pT1Blk = (PT1BLKHEADER)RTMemAllocZ(cbT1Blk);
16721687
AssertReturn(pT1Blk, VERR_NO_MEMORY);
16731688

16741689
pT1Blk->u8Nad = u8Nad;
@@ -1705,7 +1720,7 @@ static int usbCardReaderT1BlkSProcess(PUSBCARDREADER pThis, PCARDREADERSLOT pSlo
17051720
UCRLOG(("ENTER: pThis:%p, pSlot:%p, pT1BlkHeader:%.*Rhxs\n",
17061721
pThis,
17071722
pSlot,
1708-
pT1BlkHeader->u8Len + sizeof(PT1BLKHEADER) + (usbCardReaderIsCrc16ChkSum(pSlot) ? 2 : 1),
1723+
sizeof(T1BLKHEADER) + pT1BlkHeader->u8Len + (usbCardReaderIsCrc16ChkSum(pSlot) ? 2 : 1),
17091724
pT1BlkHeader));
17101725

17111726
int rc = VINF_SUCCESS;
@@ -1789,7 +1804,7 @@ static int usbCardReaderT1BlkRProcess(PUSBCARDREADER pThis, PCARDREADERSLOT pSlo
17891804
{
17901805
UCRLOG(("ENTER: pThis:%p, pSlot:%p, pT1BlkHeader:%.*Rhxs\n",
17911806
pThis, pSlot,
1792-
pT1BlkHeader->u8Len + sizeof(PT1BLKHEADER) + (usbCardReaderIsCrc16ChkSum(pSlot) ? 2 : 1),
1807+
sizeof(T1BLKHEADER) + pT1BlkHeader->u8Len + (usbCardReaderIsCrc16ChkSum(pSlot) ? 2 : 1),
17931808
pT1BlkHeader));
17941809

17951810
int rc = VINF_SUCCESS;
@@ -1867,6 +1882,38 @@ static int usbCardReaderT1BlkRProcess(PUSBCARDREADER pThis, PCARDREADERSLOT pSlo
18671882
return rc;
18681883
}
18691884

1885+
static uint8_t usbCardReaderT1ValidateBlock(PCARDREADERSLOT pSlot,
1886+
const VUSBCARDREADERBULKHDR *pCmd)
1887+
{
1888+
uint8_t enmMsgStatus = VUSBCARDREADER_MSG_STATUS_ERR_NON;
1889+
1890+
/* T1 block is T1BLKHEADER + data[T1BLKHEADER::u8Len] + checksum. */
1891+
uint32_t cbT1Block = sizeof(T1BLKHEADER) + (usbCardReaderIsCrc16ChkSum(pSlot) ? 2 : 1);
1892+
if (pCmd->dwLength < cbT1Block)
1893+
{
1894+
enmMsgStatus = VUSBCARDREADER_MSG_STATUS_ERR_XFR_OVERRUN;
1895+
}
1896+
else
1897+
{
1898+
PT1BLKHEADER pT1Hdr = (PT1BLKHEADER)&pCmd[1];
1899+
cbT1Block += pT1Hdr->u8Len;
1900+
if (pCmd->dwLength < cbT1Block)
1901+
{
1902+
enmMsgStatus = VUSBCARDREADER_MSG_STATUS_ERR_XFR_OVERRUN;
1903+
}
1904+
else
1905+
{
1906+
bool const fT1ChkSumValid = usbCardReaderT1ValidateChkSum(pSlot, (uint8_t *)&pCmd[1], pCmd->dwLength);
1907+
if (RT_UNLIKELY(!fT1ChkSumValid))
1908+
{
1909+
enmMsgStatus = VUSBCARDREADER_MSG_STATUS_ERR_XFR_PARITY_ERROR;
1910+
}
1911+
}
1912+
}
1913+
1914+
return enmMsgStatus;
1915+
}
1916+
18701917
/*
18711918
* This function process PC_to_RDR_XfrBlock (6.1.4) in T1 protocol specific way.
18721919
*/
@@ -1881,11 +1928,11 @@ static int usbCardReaderXfrBlockT1(PUSBCARDREADER pThis,
18811928

18821929
Assert(pSlot->u8ProtocolSelector == 1);
18831930

1884-
bool fT1ChkSumValid = usbCardReaderT1ValidateChkSum(pSlot, (uint8_t *)&pCmd[1], pCmd->dwLength);
1931+
uint8_t const enmMsgStatus = usbCardReaderT1ValidateBlock(pSlot, pCmd);
18851932

1886-
if (RT_UNLIKELY(!fT1ChkSumValid))
1933+
if (RT_UNLIKELY(enmMsgStatus != VUSBCARDREADER_MSG_STATUS_ERR_NON))
18871934
{
1888-
rc = uscrResponseSlotError(pThis, pSlot, VUSBCARDREADER_MSG_STATUS_ERR_XFR_PARITY_ERROR);
1935+
rc = uscrResponseSlotError(pThis, pSlot, enmMsgStatus);
18891936
}
18901937
else
18911938
{
@@ -1894,8 +1941,6 @@ static int usbCardReaderXfrBlockT1(PUSBCARDREADER pThis,
18941941
UCRLOG(("pT1Hdr->u8Len %d, pCmd->dwLength %d, pT1Hdr->u8Pcb 0x%02X\n",
18951942
pT1Hdr->u8Len, pCmd->dwLength, pT1Hdr->u8Pcb));
18961943

1897-
/** @todo validate, for example pT1Hdr->u8Len < pCmd->dwLength */
1898-
18991944
switch (pT1Hdr->u8Pcb & ISO7816_T1_BLK_TYPE_MASK)
19001945
{
19011946
case ISO7816_T1_BLK_S:
@@ -2192,84 +2237,92 @@ static int usbCardReaderDefaultPipe(PUSBCARDREADER pThis, PUSBCARDREADEREP pEp,
21922237

21932238
int rc = VINF_SUCCESS;
21942239

2195-
PVUSBSETUP pSetup = (PVUSBSETUP)pUrb->pbData;
2196-
2197-
switch (pSetup->bmRequestType & VUSB_REQ_MASK)
2240+
if (RT_UNLIKELY(pUrb->cbData < sizeof(VUSBSETUP)))
21982241
{
2199-
case VUSB_REQ_STANDARD:
2200-
if ((pSetup->bmRequestType & VUSB_DIR_MASK) == VUSB_DIR_TO_HOST)
2201-
{
2202-
switch (pSetup->bmRequestType & VUSB_RECIP_MASK)
2242+
UCRLOG(("pUrb->cbData %d\n", pUrb->cbData));
2243+
AssertFailedStmt(rc = VERR_NOT_SUPPORTED);
2244+
}
2245+
else
2246+
{
2247+
PVUSBSETUP pSetup = (PVUSBSETUP)pUrb->pbData;
2248+
2249+
switch (pSetup->bmRequestType & VUSB_REQ_MASK)
2250+
{
2251+
case VUSB_REQ_STANDARD:
2252+
if ((pSetup->bmRequestType & VUSB_DIR_MASK) == VUSB_DIR_TO_HOST)
22032253
{
2204-
case VUSB_TO_DEVICE:
2205-
rc = usbCardReaderSRToHostTodevice(pThis, pEp, pUrb, pSetup);
2206-
break;
2207-
case VUSB_TO_ENDPOINT:
2208-
case VUSB_TO_INTERFACE:
2209-
case VUSB_TO_OTHER:
2210-
default:
2211-
rc = usbCardReaderCompleteSetupUnsupported(pThis, pUrb);
2254+
switch (pSetup->bmRequestType & VUSB_RECIP_MASK)
2255+
{
2256+
case VUSB_TO_DEVICE:
2257+
rc = usbCardReaderSRToHostTodevice(pThis, pEp, pUrb, pSetup);
2258+
break;
2259+
case VUSB_TO_ENDPOINT:
2260+
case VUSB_TO_INTERFACE:
2261+
case VUSB_TO_OTHER:
2262+
default:
2263+
rc = usbCardReaderCompleteSetupUnsupported(pThis, pUrb);
2264+
}
22122265
}
2213-
}
2214-
else
2215-
{
2216-
switch (pSetup->bmRequestType & VUSB_RECIP_MASK)
2266+
else
22172267
{
2218-
case VUSB_TO_ENDPOINT:
2268+
switch (pSetup->bmRequestType & VUSB_RECIP_MASK)
22192269
{
2220-
if (pSetup->bRequest == VUSB_REQ_CLEAR_FEATURE)
2270+
case VUSB_TO_ENDPOINT:
22212271
{
2222-
UCRLOG(("endpoint:CLEAR_FEATURE: wValue %d, wIndex 0x%02X\n",
2223-
pSetup->wValue, pSetup->wIndex));
2224-
/** @todo */
2225-
unsigned i;
2226-
for (i = 0; i < RT_ELEMENTS(pThis->aEps); i++)
2272+
if (pSetup->bRequest == VUSB_REQ_CLEAR_FEATURE)
22272273
{
2228-
pThis->aEps[i].fHalted = false;
2274+
UCRLOG(("endpoint:CLEAR_FEATURE: wValue %d, wIndex 0x%02X\n",
2275+
pSetup->wValue, pSetup->wIndex));
2276+
/** @todo */
2277+
unsigned i;
2278+
for (i = 0; i < RT_ELEMENTS(pThis->aEps); i++)
2279+
{
2280+
pThis->aEps[i].fHalted = false;
2281+
}
2282+
2283+
uscrResponseCleanup(pThis);
2284+
2285+
rc = usbCardReaderCompleteOk(pThis, pUrb, pUrb->cbData);
22292286
}
2287+
else
2288+
{
2289+
rc = usbCardReaderCompleteSetupUnsupported(pThis, pUrb);
2290+
}
2291+
} break;
22302292

2231-
uscrResponseCleanup(pThis);
2232-
2233-
rc = usbCardReaderCompleteOk(pThis, pUrb, pUrb->cbData);
2234-
}
2235-
else
2236-
{
2293+
case VUSB_TO_DEVICE:
2294+
case VUSB_TO_INTERFACE:
2295+
case VUSB_TO_OTHER:
2296+
default:
22372297
rc = usbCardReaderCompleteSetupUnsupported(pThis, pUrb);
2238-
}
2239-
} break;
2240-
2241-
case VUSB_TO_DEVICE:
2242-
case VUSB_TO_INTERFACE:
2243-
case VUSB_TO_OTHER:
2244-
default:
2245-
rc = usbCardReaderCompleteSetupUnsupported(pThis, pUrb);
2298+
}
22462299
}
2247-
}
2248-
break;
2249-
case VUSB_REQ_CLASS:
2250-
if ((pSetup->bmRequestType & VUSB_DIR_MASK) == VUSB_DIR_TO_HOST)
2251-
{
2252-
switch (pSetup->bmRequestType & VUSB_RECIP_MASK)
2300+
break;
2301+
case VUSB_REQ_CLASS:
2302+
if ((pSetup->bmRequestType & VUSB_DIR_MASK) == VUSB_DIR_TO_HOST)
22532303
{
2254-
/* Linux and Windows guest make different requests? */
2255-
case VUSB_TO_DEVICE:
2256-
case VUSB_TO_INTERFACE:
2257-
rc = usbCardReaderCSToHost(pThis, pEp, pUrb, pSetup);
2258-
break;
2259-
case VUSB_TO_ENDPOINT:
2260-
case VUSB_TO_OTHER:
2261-
default:
2262-
rc = usbCardReaderCompleteSetupUnsupported(pThis, pUrb);
2304+
switch (pSetup->bmRequestType & VUSB_RECIP_MASK)
2305+
{
2306+
/* Linux and Windows guest make different requests? */
2307+
case VUSB_TO_DEVICE:
2308+
case VUSB_TO_INTERFACE:
2309+
rc = usbCardReaderCSToHost(pThis, pEp, pUrb, pSetup);
2310+
break;
2311+
case VUSB_TO_ENDPOINT:
2312+
case VUSB_TO_OTHER:
2313+
default:
2314+
rc = usbCardReaderCompleteSetupUnsupported(pThis, pUrb);
2315+
}
22632316
}
2264-
}
2265-
else
2266-
{
2267-
rc = usbCardReaderCompleteSetupUnsupported(pThis, pUrb);
2268-
}
2269-
break;
2270-
case VUSB_REQ_VENDOR:
2271-
default:
2272-
rc = usbCardReaderCompleteSetupUnsupported(pThis, pUrb);
2317+
else
2318+
{
2319+
rc = usbCardReaderCompleteSetupUnsupported(pThis, pUrb);
2320+
}
2321+
break;
2322+
case VUSB_REQ_VENDOR:
2323+
default:
2324+
rc = usbCardReaderCompleteSetupUnsupported(pThis, pUrb);
2325+
}
22732326
}
22742327

22752328
UCRLOGF(("LEAVE: rc:%Rrc\n", rc));

0 commit comments

Comments
 (0)