CVE-2024-0039 AI-generated analysis
platform/packages/modules/Bluetooth, GATT/ATT stack (system/stack/gatt/att_protocol.cc)
Adds running size accounting (size_now) with a CHECK_SIZE guard before every write in attp_build_value_cmd, so the ATT PDU builder frees the buffer and returns nullptr instead of writing past the payload_size-sized allocation. It also widens and validates the Read By Type pair-length field.
Root cause
attp_build_value_cmd allocates sizeof(BT_HDR) + payload_size + L2CAP_MIN_OFFSET. It then writes, in order: the opcode; a 1-byte pair length (GATT_RSP_READ_BY_TYPE only); a 2-byte handle (all opcodes except GATT_RSP_READ_BLOB/GATT_RSP_READ); a 2-byte offset (prepare write req/rsp only); and finally the value. It never checks that payload_size is large enough for the fixed header bytes. The only check was 'if (payload_size - p_buf->len < len) len = payload_size - p_buf->len'. When payload_size is smaller than the bytes already emitted, this subtraction is negative and the comparison is true. The negative result is assigned to the uint16_t len, which gives a large length for ARRAY_TO_STREAM and a heap overflow. The pair_len field was also a uint8_t computed as len + 2, so it was silently truncated.
How the fix works
- Commit bd2d00da33f0 / b140db65f6bc (Change-Id Ie16251c3, first landing, android-security-14.0.0_r5 and _r6), system/stack/gatt/att_protocol.cc:attp_build_value_cmd. It introduces size_t size_now (starting at 1 for the opcode) and a local CHECK_SIZE() macro: when size_now > payload_size, the macro logs 'payload size too small', calls osi_free(p_buf) and returns nullptr. CHECK_SIZE is invoked before the opcode write, after adding 1 for the pair-length byte, after adding 2 for the handle, after adding 2 for the offset, and after adding len for the value. pair_len becomes size_t and is no longer written up front. Instead it is backfilled at the end through p_pair_len, after a 'pair_len > UINT8_MAX' check that frees the buffer and returns nullptr. The value branch gains an extra 'payload_size > size_now' condition, and truncation now uses size_now instead of p_buf->len. p_buf->len is set once at the end from size_now.
- Defect in the first landing: in the GATT_RSP_READ_BY_TYPE branch it sets 'p_pair_len = p;' without advancing p (the removed UINT8_TO_STREAM used to advance it). As a result, the handle and the fields after it are written one byte early, the backfilled pair length overwrites the first handle byte, and the last byte counted in size_now is never written.
- Commit 06f9a5f1526b (Change-Id I291fd665, 'Reland', android-security-14.0.0_r6, also lists Bug 315127634) changes the same file and function and is diffed against the same pre-fix base. It uses the same size_now/CHECK_SIZE/backfill scheme, with three differences: 'p_pair_len = p++;' reserves the byte correctly; logging switches to LOG_ERROR(...); and the extra 'payload_size > size_now' condition is dropped. CHECK_SIZE already guarantees size_now <= payload_size, so payload_size - size_now cannot go negative. The reland adds 'Test: atest net_test_stack_gatt'. Per index search, stack_gatt_test.cc has tests (attp_build_value_cmd_no_p_data, _full_p_data, _partial_p_data, _p_data_null) that exercise the function through a legacy::testing declaration.
Completeness
- Every write site inside attp_build_value_cmd (opcode, pair length, handle, offset, value) is now preceded by a size check against payload_size, for every opcode path the function handles.
- The underflow is removed structurally: because CHECK_SIZE runs after each header increment, payload_size - size_now is non-negative when the value length is clamped.
- The pair_len > UINT8_MAX case now fails the build (nullptr) instead of silently truncating. The final cast of p_buf->len to uint16_t is bounded by payload_size (a uint16_t), so it cannot truncate.
- The fix changes only this function. The callers that consume its return value are not in the diff, so the material does not show whether every caller tolerates a nullptr return.
- attp_build_opcode_cmd (shown in context) allocates a fixed 1-byte payload and is unaffected. Other builders in att_protocol.cc are not shown.
Behavior changes
- attp_build_value_cmd can now return nullptr, where previously it always returned a buffer. This happens when payload_size is smaller than the header bytes the opcode requires, or when the Read By Type pair length would exceed 255.
- New error logs: 'payload size too small' and a 'pair_len greater than' UINT8_MAX message.
- The first landing (Ie16251c3) produced malformed GATT_RSP_READ_BY_TYPE PDUs because p was not advanced past the pair-length byte. The reland (I291fd665) restores the correct layout.
Per major
- Android 14: Fixed via two landings: Ie16251c3 (bd2d00da33f0 in android-security-14.0.0_r5, also b140db65f6bc in _r6), followed by the corrected Reland I291fd665 (06f9a5f1526b in android-security-14.0.0_r6). The android-security-14.0.0_r29 code shows the reland form ('p_pair_len = p++;', LOG_ERROR), which is consistent with the 7/8 line match of the patched region.
- Android 15: Not affected (not listed by the bulletin). 7/8 lines of the patched region are present at android-security-15.0.0_r17.
- Android 16: Not affected (not listed by the bulletin). 7/8 lines of the patched region are present at android-security-16.0.0_r8.
- Android 17: Not affected (not listed by the bulletin). 7/8 lines of the patched region are present at android-security-17.0.0_r1.
Unknowns
- The material does not show the production callers of attp_build_value_cmd, or how payload_size and len reach it from remote traffic (including whether payload_size derives from the negotiated ATT MTU).
- Whether callers were separately updated to handle a nullptr return is not shown.
- Why both b140db65f6bc (original) and 06f9a5f1526b (reland) appear in android-security-14.0.0_r6. The 'Reland' title implies an intervening revert, but none is shown.
- Whether Bug 315127634 tracks the missing p++ defect is not stated.
- Which form of the code is present in Android 15-17 is not shown beyond the 7/8 line match.