[Pcsclite-muscle] Insufficient checks in CCID

[Ludovic Rousseau]

Le mer. 5 août 2020 à 02:58, Maksim Ivanov <emaxx at google.com> a écrit :
>
> Hello,

Hello Maksim,

> The CCID free software driver is missing a few checks and graceful
> handling of some error cases:
>
> 1. The device's |bVoltageSupport| having none of the low three bits
> set. (The consequence of this issue is a hang in CmdPowerOn() in
> "check_again: ... goto check_again". The specs say that the other bits
> are reserved for future use, but theoretically this means that the
> three lowest bits can be unset for some devices.)

Fixed in 913d37d506f02a1c4543af9dd04d892b88acd9e0

> 2. Wrong (too big) ATR size received from the device. (Would lead to a
> buffer overrun in CmdPowerOn() when doing memmove() in the buffer:
> https://salsa.debian.org/rousseau/CCID/-/blob/4d5cbf703c268b31c734931166c52dcb9920c0fe/src/commands.c#L294
> .)

Fixed in a382095d8b19730c92969052f961a2066bc04c0f

> 3. memcmp() reading past the end-of-buffer when
> IFDHSetProtocolParameters() is called with the ATR of length exceeding
> 20 bytes. (Specifically, when comparing the ATR with |openpgp_atr|:
> https://salsa.debian.org/rousseau/CCID/-/blob/4d5cbf703c268b31c734931166c52dcb9920c0fe/src/ifdhandler.c#L1001
> .)

Fixed in 160e5a3cea4bf36bc0cb0844b51631e0f0d98042

> 4. Read of uninitialized buffer in CmdGetSlotStatus() at
> https://salsa.debian.org/rousseau/CCID/-/blob/4d5cbf703c268b31c734931166c52dcb9920c0fe/src/commands.c#L1201
> - in case when the control transfer returned only 1 instead of 3
> bytes.

Fixed in 6e2e1b5732739888079a829acbc8fade438d3868

> 5. Read of uninitialized buffer in ReadUSB() at
> https://salsa.debian.org/rousseau/CCID/-/blob/4d5cbf703c268b31c734931166c52dcb9920c0fe/src/ccid_usb.c#L912
> . (Because of the wrong ">=" size check - it should be a strict ">".)

Fixed in 72ddc000176c002555fda5e2259d73ef05027f7b

> 6. Read of uninitialized |convention| in IFDHSetProtocolParameters() -
> in case ATR_GetConvention() returned a failure on a malformed ATR.

Fixed in bab5d8aea2aed8336e1922798a8223d998f0beef

> 7. Read of uninitialized buffer in PPS_Match() at
> https://salsa.debian.org/rousseau/CCID/-/blob/4d5cbf703c268b31c734931166c52dcb9920c0fe/src/towitoko/pps.c#L101
> - in case |len_confirm| is unexpectedly small.

Fixed in 97de195c6ae6a9cefb60b2ab6822e67f8bc36d2e

> 8. Read of uninitialized |chain_parameter| in
> CmdXfrBlockAPDU_extended() - in case the reading in CCID_Receive()
> returned unexpectedly few bytes, less than CHAIN_PARAMETER_OFFSET from
> which the |chain_parameter| is read. (The problem is also partially
> caused by code in CmdXfrBlockAPDU_extended() that ignores
> IFD_ERROR_INSUFFICIENT_BUFFER:
> https://salsa.debian.org/rousseau/CCID/-/blob/4d5cbf703c268b31c734931166c52dcb9920c0fe/src/commands.c#L1701
> )

We can discuss this one.
CmdXfrBlockAPDU_extended() contains:

return_value = CCID_Receive(reader_index, &local_rx_length, rx_buffer,
&chain_parameter);
if (IFD_ERROR_INSUFFICIENT_BUFFER == return_value)

So I don't understand why you write "code in
CmdXfrBlockAPDU_extended() that ignores
IFD_ERROR_INSUFFICIENT_BUFFER:'"

I think I missed the point here.

> 9. Read of uninitialized buffer in PPS_Exchange() at
> https://salsa.debian.org/rousseau/CCID/-/blob/4d5cbf703c268b31c734931166c52dcb9920c0fe/src/towitoko/pps.c#L79
> - in case when |len_request| is unexpectedly small.

len_request is at least 3. It is set in PPS_GetLength() at
https://salsa.debian.org/rousseau/CCID/-/blob/master/src/towitoko/pps.c#L110
So your use case should never happen.
I think it is a false positive :-)

> 10. Reads of uninitialized buffer in CmdPowerOn(), CCID_Receive(),
> CmdGetSlotStatus() - in case when the read buffer is too small to
> contain the |ERROR_OFFSET| index.)

Fixed in e3376324961125bee366567e25c6613c10269866

> P.S. Posted to the public mailing list, since I don't think these
> errors are exploitable (not considering the case of a malicious
> device, but USB is fundamentally insecure anyway).

I also think they are NOT exploitable unless you use a malicious
reader or driver.
If you have a malicious driver then you have other problems since
pcscd (and the driver) is run as root.
This is a known missing feature. See
https://salsa.debian.org/rousseau/PCSC/-/issues/10

Thanks

--
 Dr. Ludovic Rousseau