[Qemu-devel] [PATCH v2 4/8] usb-ccid: Fix ATR parsing

[Stefan Fritsch]

From: Stefan Fritsch <address@hidden>

- The number of parameter set TA_i...TD_i is unlimited. The list ends
  if TD_i is not present or the high nibble is zero.
- If at least one protocol in any of the TD bytes is non-zero, the
  ATR must have a checksum.
- Add a missing length check before accessing TD.
- Fixup a wrong checksum but accept the ATR.

Signed-off-by: Stefan Fritsch <address@hidden>
Signed-off-by: Christian Ehrhardt <address@hidden>
---
 hw/usb/ccid-card-passthru.c | 34 ++++++++++++++++++++++++++--------
 1 file changed, 26 insertions(+), 8 deletions(-)

diff --git a/hw/usb/ccid-card-passthru.c b/hw/usb/ccid-card-passthru.c
index 45d96b03c6..e2e94ac1ba 100644
--- a/hw/usb/ccid-card-passthru.c
+++ b/hw/usb/ccid-card-passthru.c
@@ -146,8 +146,7 @@ static void ccid_card_vscard_handle_init(
 
 static int check_atr(PassthruState *card, uint8_t *data, int len)
 {
-    int historical_length, opt_bytes;
-    int td_count = 0;
+    int historical_length, opt_bytes, tck = 0;
     int td;
 
     if (len < 2) {
@@ -160,10 +159,8 @@ static int check_atr(PassthruState *card, uint8_t *data, 
int len)
                 data[0]);
         return 0;
     }
-    td_count = 0;
     td = data[1] >> 4;
-    while (td && td_count < 2 && opt_bytes + historical_length + 2 < len) {
-        td_count++;
+    while (td && opt_bytes + historical_length + 2 < len) {
         if (td & 0x1) {
             opt_bytes++;
         }
@@ -175,16 +172,37 @@ static int check_atr(PassthruState *card, uint8_t *data, 
int len)
         }
         if (td & 0x8) {
             opt_bytes++;
-            td = data[opt_bytes + 2] >> 4;
+            if (opt_bytes + 1 >= len) {
+                break;
+            }
+            /* Checksum byte must be present if T!=0 */
+            if (data[opt_bytes + 1] & 0xf) {
+                tck = 1;
+            }
+            td = data[opt_bytes + 1] >> 4;
+        } else {
+            td = 0;
         }
     }
-    if (len < 2 + historical_length + opt_bytes) {
+    if (len < 2 + historical_length + opt_bytes + tck) {
         DPRINTF(card, D_WARN,
             "atr too short: len %d, but historical_len %d, T1 0x%X\n",
             len, historical_length, data[1]);
         return 0;
     }
-    if (len > 2 + historical_length + opt_bytes) {
+    if (tck) {
+        int i;
+        uint8_t cksum = 0;
+        for (i = 1; i < 2 + historical_length + opt_bytes; ++i) {
+            cksum ^= data[i];
+        }
+        if (cksum != data[i]) {
+            DPRINTF(card, D_WARN, "atr has bad checksum: "
+                "real=0x%x should be 0x%x (FIXED)\n", cksum, data[i]);
+            data[i] = cksum;
+        }
+    }
+    if (len > 2 + historical_length + opt_bytes + tck) {
         DPRINTF(card, D_WARN,
             "atr too long: len %d, but hist/opt %d/%d, T1 0x%X\n",
             len, historical_length, opt_bytes, data[1]);
-- 
2.11.0