Merge "Prevent OOB read in ce_t4t_process_select_file_cmd" into qt-qpr1-dev am: a705221ae5 Change-Id: If67c117588610189c57dab083ae4d5bb7d5306a6
diff --git a/OWNERS b/OWNERS index 0aa310b..45e7662 100644 --- a/OWNERS +++ b/OWNERS
@@ -1,4 +1,4 @@ zachoverflow@google.com -rmojumder@google.com jackcwyu@google.com georgekgchang@google.com +alisher@google.com
diff --git a/src/Android.bp b/src/Android.bp index 60fb9b0..9d2ae92 100644 --- a/src/Android.bp +++ b/src/Android.bp
@@ -10,7 +10,6 @@ "liblog", "libdl", "libhardware", - "libmetricslogger", "libz", "libchrome", "libbase", @@ -18,8 +17,6 @@ // Treble configuration "libhidlbase", - "libhidltransport", - "libhwbinder", "libutils", "android.hardware.nfc@1.0", "android.hardware.nfc@1.1",
diff --git a/src/adaptation/debug_nfcsnoop.cc b/src/adaptation/debug_nfcsnoop.cc index 130e1f7..cd655cd 100644 --- a/src/adaptation/debug_nfcsnoop.cc +++ b/src/adaptation/debug_nfcsnoop.cc
@@ -54,7 +54,7 @@ while (ringbuffer_available(buffer) < (length + sizeof(nfcsnooz_header_t))) { ringbuffer_pop(buffer, (uint8_t*)&header, sizeof(nfcsnooz_header_t)); - ringbuffer_delete(buffer, header.length - 1); + ringbuffer_delete(buffer, header.length); } // Insert data
diff --git a/src/gki/ulinux/gki_ulinux.cc b/src/gki/ulinux/gki_ulinux.cc index fc3b5c6..f2e6b64 100644 --- a/src/gki/ulinux/gki_ulinux.cc +++ b/src/gki/ulinux/gki_ulinux.cc
@@ -118,8 +118,6 @@ pthread_mutexattr_t attr; tGKI_OS* p_os; - memset(&gki_cb, 0, sizeof(gki_cb)); - gki_buffer_init(); gki_timers_init(); gki_cb.com.OSTicks = (uint32_t)times(nullptr);
diff --git a/src/include/nfc_config.h b/src/include/nfc_config.h index 68e2d88..dda4260 100644 --- a/src/include/nfc_config.h +++ b/src/include/nfc_config.h
@@ -27,6 +27,7 @@ #define NAME_POLLING_TECH_MASK "POLLING_TECH_MASK" #define NAME_P2P_LISTEN_TECH_MASK "P2P_LISTEN_TECH_MASK" #define NAME_UICC_LISTEN_TECH_MASK "UICC_LISTEN_TECH_MASK" +#define NAME_HOST_LISTEN_TECH_MASK "HOST_LISTEN_TECH_MASK" #define NAME_NFA_DM_CFG "NFA_DM_CFG" #define NAME_SCREEN_OFF_POWER_STATE "SCREEN_OFF_POWER_STATE" #define NAME_NFA_MAX_EE_SUPPORTED "NFA_MAX_EE_SUPPORTED"
diff --git a/src/nfc/nfc/nfc_ncif.cc b/src/nfc/nfc/nfc_ncif.cc index 0322a97..dfb9931 100644 --- a/src/nfc/nfc/nfc_ncif.cc +++ b/src/nfc/nfc/nfc_ncif.cc
@@ -26,7 +26,6 @@ #include <android-base/stringprintf.h> #include <base/logging.h> #include <log/log.h> -#include <metricslogger/metrics_logger.h> #include "nfc_target.h" @@ -539,7 +538,6 @@ void nfc_ncif_event_status(tNFC_RESPONSE_EVT event, uint8_t status) { tNFC_RESPONSE evt_data; if (event == NFC_NFCC_TIMEOUT_REVT && status == NFC_STATUS_HW_TIMEOUT) { - android::metricslogger::LogCounter("nfc_hw_timeout_error", 1); uint32_t cmd_hdr = (nfc_cb.last_hdr[0] << 8) | nfc_cb.last_hdr[1]; android::util::stats_write(android::util::NFC_ERROR_OCCURRED, (int32_t)NCI_TIMEOUT, (int32_t)cmd_hdr, @@ -570,23 +568,6 @@ } android::util::stats_write(android::util::NFC_ERROR_OCCURRED, (int32_t)ERROR_NTF, (int32_t)0, (int32_t)status); - - if (status == NFC_STATUS_TIMEOUT) - android::metricslogger::LogCounter("nfc_rf_timeout_error", 1); - else if (status == NFC_STATUS_EE_TIMEOUT) - android::metricslogger::LogCounter("nfc_ee_timeout_error", 1); - else if (status == NFC_STATUS_ACTIVATION_FAILED) - android::metricslogger::LogCounter("nfc_rf_activation_failed", 1); - else if (status == NFC_STATUS_EE_INTF_ACTIVE_FAIL) - android::metricslogger::LogCounter("nfc_ee_activation_failed", 1); - else if (status == NFC_STATUS_RF_TRANSMISSION_ERR) - android::metricslogger::LogCounter("nfc_rf_transmission_error", 1); - else if (status == NFC_STATUS_EE_TRANSMISSION_ERR) - android::metricslogger::LogCounter("nfc_ee_transmission_error", 1); - else if (status == NFC_STATUS_RF_PROTOCOL_ERR) - android::metricslogger::LogCounter("nfc_rf_protocol_error", 1); - else if (status == NFC_STATUS_EE_PROTOCOL_ERR) - android::metricslogger::LogCounter("nfc_ee_protocol_error", 1); } /*******************************************************************************
diff --git a/src/nfc/tags/rw_t3t.cc b/src/nfc/tags/rw_t3t.cc index 6ac933d..6613dfc 100644 --- a/src/nfc/tags/rw_t3t.cc +++ b/src/nfc/tags/rw_t3t.cc
@@ -1322,15 +1322,28 @@ p_cb->ndef_attrib.nbr, p_cb->ndef_attrib.nbw, p_cb->ndef_attrib.nmaxb, p_cb->ndef_attrib.writef, p_cb->ndef_attrib.rwflag, p_cb->ndef_attrib.ln); - - /* Set data for RW_T3T_NDEF_DETECT_EVT */ - evt_data.status = p_cb->ndef_attrib.status; - evt_data.cur_size = p_cb->ndef_attrib.ln; - evt_data.max_size = (uint32_t)p_cb->ndef_attrib.nmaxb * 16; - evt_data.protocol = NFC_PROTOCOL_T3T; - evt_data.flags = (RW_NDEF_FL_SUPPORTED | RW_NDEF_FL_FORMATED); - if (p_cb->ndef_attrib.rwflag == T3T_MSG_NDEF_RWFLAG_RO) - evt_data.flags |= RW_NDEF_FL_READ_ONLY; + if (p_cb->ndef_attrib.nbr > T3T_MSG_NUM_BLOCKS_CHECK_MAX || + p_cb->ndef_attrib.nbw > T3T_MSG_NUM_BLOCKS_UPDATE_MAX) { + /* It would result in CHECK Responses exceeding the maximum length + * of an NFC-F Frame */ + LOG(ERROR) << StringPrintf( + "Unsupported NDEF Attributes value: Nbr=%i, Nbw=%i, Nmaxb=%i," + "WriteF=%i, RWFlag=%i, Ln=%i", + p_cb->ndef_attrib.nbr, p_cb->ndef_attrib.nbw, + p_cb->ndef_attrib.nmaxb, p_cb->ndef_attrib.writef, + p_cb->ndef_attrib.rwflag, p_cb->ndef_attrib.ln); + p_cb->ndef_attrib.status = NFC_STATUS_FAILED; + evt_data.status = NFC_STATUS_BAD_RESP; + } else { + /* Set data for RW_T3T_NDEF_DETECT_EVT */ + evt_data.status = p_cb->ndef_attrib.status; + evt_data.cur_size = p_cb->ndef_attrib.ln; + evt_data.max_size = (uint32_t)p_cb->ndef_attrib.nmaxb * 16; + evt_data.protocol = NFC_PROTOCOL_T3T; + evt_data.flags = (RW_NDEF_FL_SUPPORTED | RW_NDEF_FL_FORMATED); + if (p_cb->ndef_attrib.rwflag == T3T_MSG_NDEF_RWFLAG_RO) + evt_data.flags |= RW_NDEF_FL_READ_ONLY; + } } } }
diff --git a/utils/Android.bp b/utils/Android.bp index 04053a4..4a35cc0 100644 --- a/utils/Android.bp +++ b/utils/Android.bp
@@ -48,3 +48,17 @@ "libbase", ], } + +cc_fuzz { + name: "nfc_utils_ringbuffer_fuzzer", + host_supported: true, + srcs: [ + "test/ringbuffer_fuzzer/ringbuffer_fuzzer.cpp", + ], + static_libs: [ + "libnfcutils", + ], + corpus: [ + "test/ringbuffer_fuzzer/corpus/*", + ], +} \ No newline at end of file
diff --git a/utils/ringbuffer.cc b/utils/ringbuffer.cc index e130afd..7b494a3 100644 --- a/utils/ringbuffer.cc +++ b/utils/ringbuffer.cc
@@ -30,9 +30,11 @@ }; ringbuffer_t* ringbuffer_init(const size_t size) { + if (size == 0) return nullptr; + ringbuffer_t* p = static_cast<ringbuffer_t*>(calloc(1, sizeof(ringbuffer_t))); - if (p == nullptr) return p; + if (p == nullptr) return nullptr; p->base = static_cast<uint8_t*>(calloc(size, sizeof(uint8_t))); p->head = p->tail = p->base;
diff --git a/utils/test/ringbuffer_fuzzer/corpus/zero_size_ringbuffer_fpe b/utils/test/ringbuffer_fuzzer/corpus/zero_size_ringbuffer_fpe new file mode 100644 index 0000000..b9f6f03 --- /dev/null +++ b/utils/test/ringbuffer_fuzzer/corpus/zero_size_ringbuffer_fpe Binary files differ
diff --git a/utils/test/ringbuffer_fuzzer/ringbuffer_fuzzer.cpp b/utils/test/ringbuffer_fuzzer/ringbuffer_fuzzer.cpp new file mode 100644 index 0000000..9523fe5 --- /dev/null +++ b/utils/test/ringbuffer_fuzzer/ringbuffer_fuzzer.cpp
@@ -0,0 +1,83 @@ +#include <sys/types.h> + +#include <algorithm> +#include <cstddef> +#include <cstdint> +#include <cstdlib> + +#include "ringbuffer.h" + +extern "C" int LLVMFuzzerTestOneInput(const uint8_t* Data, size_t Size) { + if (Size < 2) { + return 0; + } + + // Only allocate up to 1 << 16 bytes of memory. We shouldn't ever be + // exercising more than this. + uint16_t buffer_size = *((const uint16_t*)Data); + ringbuffer_t* buffer = ringbuffer_init(buffer_size); + + if (buffer == nullptr) { + return 0; + } + + for (size_t i = 2; i < Size;) { + size_t bytes_left = Size - i - 1; + switch (Data[i++] % 6) { + case 0: { + ringbuffer_available(buffer); + break; + } + case 1: { + ringbuffer_size(buffer); + break; + } + case 2: { + if (bytes_left < 2) { + break; + } + + size_t bytes_to_insert = std::min(bytes_left - 1, (size_t)Data[i++]); + ringbuffer_insert(buffer, &Data[i], bytes_to_insert); + i += bytes_to_insert; + break; + } + case 3: { + if (bytes_left < 2) { + break; + } + + size_t bytes_to_grab = Data[i++]; + uint8_t* copy_buffer = (uint8_t*)malloc(bytes_to_grab); + off_t offset = 0; + if (ringbuffer_size(buffer) != 0) { + offset = Data[i++] % ringbuffer_size(buffer); + } + + ringbuffer_peek(buffer, offset, copy_buffer, (size_t)bytes_to_grab); + free(copy_buffer); + break; + } + case 4: { + if (bytes_left < 1) { + break; + } + + size_t bytes_to_grab = Data[i++]; + uint8_t* copy_buffer = (uint8_t*)malloc(bytes_to_grab); + ringbuffer_pop(buffer, copy_buffer, bytes_to_grab); + free(copy_buffer); + break; + } + case 5: { + if (bytes_left < 1) { + break; + } + ringbuffer_delete(buffer, (size_t)Data[i++]); + } + } + } + + ringbuffer_free(buffer); + return 0; +}