commit 5a5cbd9f6507fc2d0b37c4d392476228e8581b3c
author George Chang <georgekgchang@google.com> Mon Jan 28 10:17:25 2019 +0800
committer Arjun Garg <arjgarg@google.com> Tue Jul 16 15:20:27 2019 -0700
tree 6163aec5d3a117415fd791e3823f5c9a45cec532
parent 0a7432f80ec588cb0f781c06e818b71cf93a3115

Prevent OOB read in rw_t4t.cc

Bug: 120865977
Bug: 120274615
Bug: 124462242
Test: Read T4T Tag
Exempt-From-Owner-Approval: Old Owners are all transferred to another BU
Merged-In: Id1b07fa79edabaf27e18a7147d1e7ae66cf3c4b3
Change-Id: Id1b07fa79edabaf27e18a7147d1e7ae66cf3c4b3
(cherry picked from commit 38e3ecb36cb2716cc1e722305b42c2db0d0d01fc)

src/nfc/include/tags_defs.h

diff --git a/src/nfc/include/tags_defs.h b/src/nfc/include/tags_defs.h
index d7adc60..55022ff 100644
--- a/src/nfc/include/tags_defs.h
+++ b/src/nfc/include/tags_defs.h
@@ -436,6 +436,7 @@
 
 #define T4T_FILE_LENGTH_SIZE 0x02
 #define T4T_ADDI_FRAME_RESP 0xAFU
+#define T4T_DES_GET_VERSION_LEN 0x09
 #define T4T_SIZE_IDENTIFIER_2K 0x16U
 #define T4T_SIZE_IDENTIFIER_4K 0x18U
 #define T4T_SIZE_IDENTIFIER_8K 0x1AU

src/nfc/tags/rw_t4t.cc

diff --git a/src/nfc/tags/rw_t4t.cc b/src/nfc/tags/rw_t4t.cc
index fd0bbdf..f9549d0 100644
--- a/src/nfc/tags/rw_t4t.cc
+++ b/src/nfc/tags/rw_t4t.cc
@@ -22,6 +22,7 @@
  *  mode.
  *
  ******************************************************************************/
+#include <log/log.h>
 #include <string.h>
 
 #include <android-base/stringprintf.h>
@@ -227,6 +228,12 @@
   uint8_t* p;
   uint16_t major_version, minor_version;
 
+  if (p_r_apdu->len < T4T_DES_GET_VERSION_LEN) {
+    LOG(ERROR) << StringPrintf("%s incorrect p_r_apdu length", __func__);
+    android_errorWriteLog(0x534e4554, "120865977");
+    return false;
+  }
+
   p = (uint8_t*)(p_r_apdu + 1) + p_r_apdu->offset;
   major_version = *(p + 3);
   minor_version = *(p + 4);
@@ -1008,6 +1015,8 @@
 
     rw_data.t4t_sw.sw1 = sw1;
     rw_data.t4t_sw.sw2 = sw2;
+    rw_data.ndef.cur_size = 0;
+    rw_data.ndef.max_size = 0;
 
     switch (p_t4t->state) {
       case RW_T4T_STATE_DETECT_NDEF:
@@ -1789,18 +1798,28 @@
       "RW T4T state: <%s (%d)>", rw_t4t_get_state_name(p_t4t->state).c_str(),
       p_t4t->state);
 
+  if (p_t4t->state != RW_T4T_STATE_IDLE &&
+      p_t4t->state != RW_T4T_STATE_PRESENCE_CHECK &&
+      p_r_apdu->len < T4T_RSP_STATUS_WORDS_SIZE) {
+    LOG(ERROR) << StringPrintf("%s incorrect p_r_apdu length", __func__);
+    android_errorWriteLog(0x534e4554, "120865977");
+    rw_t4t_handle_error(NFC_STATUS_FAILED, 0, 0);
+    GKI_freebuf(p_r_apdu);
+    return;
+  }
+
   switch (p_t4t->state) {
     case RW_T4T_STATE_IDLE:
-/* Unexpected R-APDU, it should be raw frame response */
-/* forward to upper layer without parsing */
-DLOG_IF(INFO, nfc_debug_enabled)
-    << StringPrintf("RW T4T Raw Frame: Len [0x%X] Status [%s]", p_r_apdu->len,
-                    NFC_GetStatusName(p_data->data.status).c_str());
-if (rw_cb.p_cback) {
-  rw_data.raw_frame.status = p_data->data.status;
-  rw_data.raw_frame.p_data = p_r_apdu;
-  (*(rw_cb.p_cback))(RW_T4T_RAW_FRAME_EVT, &rw_data);
-  p_r_apdu = NULL;
+      /* Unexpected R-APDU, it should be raw frame response */
+      /* forward to upper layer without parsing */
+      DLOG_IF(INFO, nfc_debug_enabled) << StringPrintf(
+          "RW T4T Raw Frame: Len [0x%X] Status [%s]", p_r_apdu->len,
+          NFC_GetStatusName(p_data->data.status).c_str());
+      if (rw_cb.p_cback) {
+        rw_data.raw_frame.status = p_data->data.status;
+        rw_data.raw_frame.p_data = p_r_apdu;
+        (*(rw_cb.p_cback))(RW_T4T_RAW_FRAME_EVT, &rw_data);
+        p_r_apdu = NULL;
       } else {
         GKI_freebuf(p_r_apdu);
       }