commit 0e0664b1272d6142b5ea556898fab41a901e8231
author Andrew Scull <ascull@google.com> Tue May 03 21:12:53 2022 +0000
committer CQ Bot Account <pigweed-scoped@luci-project-accounts.iam.gserviceaccount.com> Fri May 06 23:11:36 2022 +0000
tree ce3c9689e5f176af1f33a4f714d09efacf431614
parent 1f2716bb0d78febeb5e738b4288ec4742f2c209a

Don't read beyond end of BccHandover

Check that there are still map entries remaining before trying to read
further entries. Allow, but ignore, unknown entries for forward
compatibility.

Change-Id: I71be0b480c83d65ddb3cb5a087e0586fb817ae02
Reviewed-on: https://pigweed-review.googlesource.com/c/open-dice/+/93560
Reviewed-by: Hasini Gunasinghe <hasinitg@google.com>
Commit-Queue: Darren Krahn <dkrahn@google.com>
Reviewed-by: Darren Krahn <dkrahn@google.com>

src/android/bcc.c

diff --git a/src/android/bcc.c b/src/android/bcc.c
index 8855f71..d58e80a 100644
--- a/src/android/bcc.c
+++ b/src/android/bcc.c
@@ -215,9 +215,10 @@
   // }
   struct CborIn in;
   int64_t label;
+  size_t num_pairs;
   size_t item_size;
   CborInInit(bcc_handover, bcc_handover_size, &in);
-  if (CborReadMap(&in, &item_size) != CBOR_READ_RESULT_OK || item_size < 2 ||
+  if (CborReadMap(&in, &num_pairs) != CBOR_READ_RESULT_OK || num_pairs < 2 ||
       // Read the attestation CDI.
       CborReadInt(&in, &label) != CBOR_READ_RESULT_OK ||
       label != kCdiAttestLabel ||
@@ -233,17 +234,16 @@
   }
 
   size_t bcc_size = 0;
-  // Calculate the BCC size, if the BCC is present in the BccHandover.
-  if (CborReadInt(&in, &label) == CBOR_READ_RESULT_OK) {
-    if (label != kBccLabel) {
-      return kDiceResultInvalidInput;
+  if (num_pairs >= 3 && CborReadInt(&in, &label) == CBOR_READ_RESULT_OK) {
+    if (label == kBccLabel) {
+      // Calculate the BCC size, if the BCC is present in the BccHandover.
+      size_t bcc_start = CborInOffset(&in);
+      if (CborReadSkip(&in) != CBOR_READ_RESULT_OK) {
+        return kDiceResultInvalidInput;
+      }
+      bcc = bcc_handover + bcc_start;
+      bcc_size = CborInOffset(&in) - bcc_start;
     }
-    size_t bcc_start = CborInOffset(&in);
-    bcc = bcc_handover + bcc_start;
-    if (CborReadSkip(&in) != CBOR_READ_RESULT_OK) {
-      return kDiceResultInvalidInput;
-    }
-    bcc_size = CborInOffset(&in) - bcc_start;
   }
 
   // Write the new handover data.

src/android/bcc_test.cc

diff --git a/src/android/bcc_test.cc b/src/android/bcc_test.cc
index 44dce5b..397eee7 100644
--- a/src/android/bcc_test.cc
+++ b/src/android/bcc_test.cc
@@ -115,7 +115,7 @@
                       sizeof(bcc_handover) - 8 - 73));
 }
 
-TEST(BccHandoverNoCertTest, InHandoverWithoutBccOutHandoverWithBcc) {
+TEST(BccHandoverTest, InHandoverWithoutBccOutHandoverWithBcc) {
   const uint8_t bcc_handover[] = {
       0xa2,
       // CDI attest
@@ -126,7 +126,34 @@
       0x02, 0x58, 0x20, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
       0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
       0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-  };
+      // 8-bytes of trailing data that aren't part of the BCC.
+      0x00, 0x41, 0x55, 0xa0, 0x42, 0x11, 0x22, 0x40};
+  DiceInputValues input_values = {};
+  uint8_t next_bcc_handover[1024] = {};
+  size_t next_bcc_handover_size;
+  DiceResult result = BccHandoverMainFlow(
+      /*context=*/NULL, bcc_handover, sizeof(bcc_handover), &input_values,
+      sizeof(next_bcc_handover), next_bcc_handover, &next_bcc_handover_size);
+  EXPECT_EQ(kDiceResultOk, result);
+  EXPECT_GT(next_bcc_handover_size, sizeof(bcc_handover));
+  EXPECT_EQ(0xa3, next_bcc_handover[0]);
+}
+
+TEST(BccHandoverTest, InHandoverWithoutBccButUnknownFieldOutHandoverWithBcc) {
+  const uint8_t bcc_handover[] = {
+      0xa3,
+      // CDI attest
+      0x01, 0x58, 0x20, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+      0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+      0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+      // CDI seal
+      0x02, 0x58, 0x20, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+      0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+      0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+      // Ignored unknown field
+      0x04, 0x01,
+      // 8-bytes of trailing data that aren't part of the BCC.
+      0x00, 0x41, 0x55, 0xa0, 0x42, 0x11, 0x22, 0x40};
   DiceInputValues input_values = {};
   uint8_t next_bcc_handover[1024] = {};
   size_t next_bcc_handover_size;