Prevent Out of bounds read in llcp code Modifies llcp_util.cc & llcp_link.cc to fix the OOB issues. Test: Nfc Enable/Disable, Android Beam, Tag reading Bug: 114237888 Bug: 114238578 Bug: 111699773 Bug: 111660010 Merged-In: Ie28888ddb9a5a2605ae32a27e3dba01c35ab3913 Change-Id: Ie28888ddb9a5a2605ae32a27e3dba01c35ab3913 (cherry picked from commit 23eb85a028780af14f617e27938540a52c18a8d5)
diff --git a/src/nfc/llcp/llcp_link.cc b/src/nfc/llcp/llcp_link.cc index cebabba..8a5ddf2 100644 --- a/src/nfc/llcp/llcp_link.cc +++ b/src/nfc/llcp/llcp_link.cc
@@ -1131,7 +1131,7 @@ agf_length = p_agf->len; p = (uint8_t*)(p_agf + 1) + p_agf->offset; - while (agf_length > 0) { + while (agf_length >= LLCP_PDU_HEADER_SIZE) { /* get length of PDU */ p_pdu_length = p; BE_STREAM_TO_UINT16(pdu_length, p);
diff --git a/src/nfc/llcp/llcp_util.cc b/src/nfc/llcp/llcp_util.cc index 8b582d8..973a1b9 100644 --- a/src/nfc/llcp/llcp_util.cc +++ b/src/nfc/llcp/llcp_util.cc
@@ -26,6 +26,7 @@ #include <android-base/stringprintf.h> #include <base/logging.h> +#include <log/log.h> #include "bt_types.h" #include "gki.h" @@ -49,20 +50,24 @@ bool llcp_util_parse_link_params(uint16_t length, uint8_t* p_bytes) { uint8_t param_type, param_len, *p = p_bytes; - while (length) { + while (length >= 2) { BE_STREAM_TO_UINT8(param_type, p); - length--; + BE_STREAM_TO_UINT8(param_len, p); + if (length < param_len + 2) { + android_errorWriteLog(0x534e4554, "114238578"); + LOG(ERROR) << StringPrintf("Bad LTV's"); + return false; + } + length -= param_len + 2; switch (param_type) { case LLCP_VERSION_TYPE: - BE_STREAM_TO_UINT8(param_len, p); BE_STREAM_TO_UINT8(llcp_cb.lcb.peer_version, p); DLOG_IF(INFO, nfc_debug_enabled) << StringPrintf("Peer Version - 0x%02X", llcp_cb.lcb.peer_version); break; case LLCP_MIUX_TYPE: - BE_STREAM_TO_UINT8(param_len, p); BE_STREAM_TO_UINT16(llcp_cb.lcb.peer_miu, p); llcp_cb.lcb.peer_miu &= LLCP_MIUX_MASK; llcp_cb.lcb.peer_miu += LLCP_DEFAULT_MIU; @@ -71,14 +76,12 @@ break; case LLCP_WKS_TYPE: - BE_STREAM_TO_UINT8(param_len, p); BE_STREAM_TO_UINT16(llcp_cb.lcb.peer_wks, p); DLOG_IF(INFO, nfc_debug_enabled) << StringPrintf("Peer WKS - 0x%04X", llcp_cb.lcb.peer_wks); break; case LLCP_LTO_TYPE: - BE_STREAM_TO_UINT8(param_len, p); BE_STREAM_TO_UINT8(llcp_cb.lcb.peer_lto, p); llcp_cb.lcb.peer_lto *= LLCP_LTO_UNIT; /* 10ms unit */ DLOG_IF(INFO, nfc_debug_enabled) @@ -86,7 +89,6 @@ break; case LLCP_OPT_TYPE: - BE_STREAM_TO_UINT8(param_len, p); BE_STREAM_TO_UINT8(llcp_cb.lcb.peer_opt, p); DLOG_IF(INFO, nfc_debug_enabled) << StringPrintf("Peer OPT - 0x%02X", llcp_cb.lcb.peer_opt); @@ -94,17 +96,9 @@ default: LOG(ERROR) << StringPrintf("Unexpected type 0x%x", param_type); - BE_STREAM_TO_UINT8(param_len, p); p += param_len; break; } - - if (length >= param_len + 1) - length -= param_len + 1; - else { - LOG(ERROR) << StringPrintf("Bad LTV's"); - return false; - } } return true; } @@ -474,13 +468,19 @@ p_params->sn[0] = 0; p_params->sn[1] = 0; - while (length) { + while (length >= 2) { BE_STREAM_TO_UINT8(param_type, p); - length--; + BE_STREAM_TO_UINT8(param_len, p); + /* check remaining lengh */ + if (length < param_len + 2) { + android_errorWriteLog(0x534e4554, "111660010"); + LOG(ERROR) << StringPrintf("Bad LTV's"); + return LLCP_STATUS_FAIL; + } + length -= param_len + 2; switch (param_type) { case LLCP_MIUX_TYPE: - BE_STREAM_TO_UINT8(param_len, p); BE_STREAM_TO_UINT16(p_params->miu, p); p_params->miu &= LLCP_MIUX_MASK; p_params->miu += LLCP_DEFAULT_MIU; @@ -490,7 +490,6 @@ break; case LLCP_RW_TYPE: - BE_STREAM_TO_UINT8(param_len, p); BE_STREAM_TO_UINT8(p_params->rw, p); p_params->rw &= 0x0F; @@ -499,8 +498,6 @@ break; case LLCP_SN_TYPE: - BE_STREAM_TO_UINT8(param_len, p); - if (param_len == 0) { /* indicate that SN type is included without SN */ p_params->sn[1] = LLCP_SN_TYPE; @@ -519,18 +516,9 @@ default: LOG(ERROR) << StringPrintf("Unexpected type 0x%x", param_type); - BE_STREAM_TO_UINT8(param_len, p); p += param_len; break; } - - /* check remaining lengh */ - if (length >= param_len + 1) { - length -= param_len + 1; - } else { - LOG(ERROR) << StringPrintf("Bad LTV's"); - return LLCP_STATUS_FAIL; - } } return LLCP_STATUS_SUCCESS; } @@ -607,13 +595,18 @@ *p_miu = LLCP_DEFAULT_MIU; *p_rw = LLCP_DEFAULT_RW; - while (length) { + while (length >= 2) { BE_STREAM_TO_UINT8(param_type, p); - length--; + BE_STREAM_TO_UINT8(param_len, p); + if (length < param_len + 2) { + android_errorWriteLog(0x534e4554, "114237888"); + LOG(ERROR) << StringPrintf("Bad LTV's"); + return LLCP_STATUS_FAIL; + } + length -= param_len + 2; switch (param_type) { case LLCP_MIUX_TYPE: - BE_STREAM_TO_UINT8(param_len, p); BE_STREAM_TO_UINT16((*p_miu), p); (*p_miu) &= LLCP_MIUX_MASK; (*p_miu) += LLCP_DEFAULT_MIU; @@ -623,7 +616,6 @@ break; case LLCP_RW_TYPE: - BE_STREAM_TO_UINT8(param_len, p); BE_STREAM_TO_UINT8((*p_rw), p); (*p_rw) &= 0x0F; @@ -633,17 +625,9 @@ default: LOG(ERROR) << StringPrintf("Unexpected type 0x%x", param_type); - BE_STREAM_TO_UINT8(param_len, p); p += param_len; break; } - - if (length >= param_len + 1) - length -= param_len + 1; - else { - LOG(ERROR) << StringPrintf("Bad LTV's"); - return LLCP_STATUS_FAIL; - } } return LLCP_STATUS_SUCCESS; }