APDU parser has multiple errors
- Dominant language
- Rust
- Stars
- 3.4k
- Forks
- 338
- Avg merge
- 9m
- Merged PRs (30d)
- 1
Description
I found a couple of bugs while reviewing the OpenSK APDU parser, because while writing a FIDO client library, I noticed many FIDO implementations have buggy ISO 7816 implementations. I haven't run this on "real hardware" to try it out properly.
1. APDU parser does not handle extended form with Nc = 0 and Ne > 0 ("Case 2E"). [It would error out with WRONG_LENGTH](https://github.com/google/OpenSK/blob/1b70583243a45392ca6638e891b3bbb965fd9be5/src/ctap/apdu.rs#L172-L176).
Example request, where Nc = 0 and Ne = 0x1234:
```
CLA INS P1 P2 0x00 0x12 0x34
```
U2F GET_VERSION is impacted by this, other U2F commands and CTAP2 (over an ISO 7816 transport) always have command payloads.
The parser appears to only handle "Case 2S" (short form Nc = 0 and Ne > 0), but supporting extended form is required for all transports, and supporting short form is only required over NFC.
2. APDU parser accepts _malformed_ 3 byte Lc + 3 byte Le.
Per ISO 7816-4:2005 Section 5.1 (Command-response pairs), when Nc > 0 and Ne > 0 and using extended form ("Case 4E"), the correct representation is:
```
CLA INS P1 P2 0x00 {Nc as uint16_be} {command bytes} {Ne as uint16_be}
```
[The implementation here](https://github.com/google/OpenSK/blob/1b70583243a45392ca6638e891b3bbb965fd9be5/src/ctap/apdu.rs#L212-L224) appears to _also_ incorrectly accept _malformed_ requests of the form:
```
CLA INS P1 P2 0x00 {Nc as uint16_be} {command bytes} 0x00 {Ne as uint16_be}
```
The only reason Le should be prefixed with `0x00` is if the Lc field is **absent** and Le is in extended form ("Case 2E").
Accepting malformed requests can cause implementers "testing against OpenSK" to make mistakes which are only exposed when testing with another FIDO implementation which follows ISO 7816 correctly. I've already seen subtle differences in existing FIDO certified devices with malformed inputs.
**Note:** "Case 2E", "Case 2S" and "Case 4E" references to ISO 7816-3:2006 section 12.1.3 (Decoding conventions for command APDUs); S = short form, E = extended form.
Case | Command Data Field | Response Length Field | Total length
----- | ------ | ---- | -----
Case 1 | None | None | 4 bytes
Case 2S | None | Present (1 byte) | 5 bytes
Case 2E | None | Present (3 bytes) | 7 bytes
Case 3S | Present (1 byte) | None | 5 + (command data length) bytes
Case 3E | Present (3 bytes) | None | 7 + (command data length) bytes
Case 4S | Present (1 byte) | Present (1 byte) | 6 + (command data length) bytes
Case 4E | Present (3 bytes) | Present (2 bytes) | 9 + (command data length) bytes
Contributor guide
Assessment
This issue has not been assessed yet.