[Security] Patch for stack buffer overflow and heap OOB read in avrcp.c
nathan auvray <[email protected]>
| Newsgroups | org.kernel.vger.linux-bluetooth |
|---|---|
| Message-ID | <CAB6rVYfCF5+jpSTtZ8u+S4LxCZ+kGudm3OWU=-fCMbZJnYDZUg@mail.gmail.com> |
Hi, Here is a 2-patch series fixing two security vulnerabilities in AVRCP profile response handling (profiles/audio/avrcp.c): 1. Fix stack buffer overflow in avrcp_list_player_attributes_rsp 2. Fix heap out-of-bounds read in parse_media_element Best regards, Rapido =)
0001-avrcp-Fix-stack-buffer-overflow.patch
(application/octet-stream, 944 B)
From: Security Fix <[email protected]> Subject: [PATCH 1/2] avrcp: Fix stack buffer overflow in avrcp_list_player_attributes_rsp Check that count does not exceed the size of the attrs array (4 bytes) when parsing attributes received from the peer. Otherwise an attacker can write up to 255 bytes into the 4-byte stack array, causing a stack buffer overflow. Fixes: 6260f507a5 ("avrcp: Fix sending invalid attributes in GetCurrentPlayerValue") --- profiles/audio/avrcp.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c index 2194a9135..83e12057c 100644 --- a/profiles/audio/avrcp.c +++ b/profiles/audio/avrcp.c @@ -2415,6 +2415,9 @@ static gboolean avrcp_list_player_attributes_rsp(struct avctp *conn, pdu->params[i + 1] > AVRCP_ATTRIBUTE_LAST) continue; + if (count >= sizeof(attrs)) + break; + attrs[count++] = pdu->params[i + 1]; } --
0002-avrcp-Fix-heap-out-of-bounds-read.patch
(application/octet-stream, 898 B)
From: Security Fix <[email protected]> Subject: [PATCH 2/2] avrcp: Fix heap out-of-bounds read in parse_media_element Validate that the received item buffer length (len) is at least 14 + namesize before indexing operands[13 + namesize]. Otherwise, a malicious peer sending a large namesize can trigger a heap out-of-bounds read. --- profiles/audio/avrcp.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c index 2194a9135..83e12057c 100644 --- a/profiles/audio/avrcp.c +++ b/profiles/audio/avrcp.c @@ -2628,6 +2628,9 @@ static struct media_item *parse_media_element(struct avrcp *session, memset(name, 0, sizeof(name)); namesize = get_be16(&operands[11]); + if (len < 14 + namesize) + return NULL; + namelen = MIN(namesize, sizeof(name) - 1); if (namelen > 0) { memcpy(name, &operands[13], namelen); --
DISCLOSURE_REPORT.md
(text/markdown, 5 KB)
# BlueZ Security Disclosure Report
> **Target:** BlueZ (`profiles/audio/avrcp.c`)
> **Affected Version:** Upstream HEAD (`30db66dc9`, tag `5.87-19-g30db66dc9`) & older versions
---
## Executive Summary
Two security vulnerabilities were identified in the BlueZ AVRCP profile handler (`profiles/audio/avrcp.c`):
1. **Finding #1**: Double Stack Buffer Overflow in `avrcp_list_player_attributes_rsp()`. A 12-year-old regression in bounds checking allows an attacker to write up to 251 bytes past a 4-byte stack array `attrs[]`, and subsequently trigger a second stack overflow in `avrcp_get_current_player_value()`.
2. **Finding #2**: Heap Out-Of-Bounds Read in `parse_media_element()`. The `namesize` field from an incoming PDU is not validated against the item buffer length, causing `memcpy` and array indexing to read unmapped/heap bytes out-of-bounds.
---
## Vulnerability Details
### Finding #1: Double Stack Buffer Overflow in `avrcp_list_player_attributes_rsp()`
- **File**: `profiles/audio/avrcp.c:2391-2424, 2369-2389`
- **Vulnerability Type**: Stack Buffer Overflow (`WRITE`)
#### Vulnerable Code:
```c
/* avrcp_list_player_attributes_rsp() */
uint8_t attrs[AVRCP_ATTRIBUTE_LAST]; /* 4 bytes on stack */
uint8_t len, count = 0;
len = pdu->params[0]; /* Attacker controlled (0..255) */
/* Dead code check since commit 6260f507a5 (Feb 2014): count is always 0 here! */
if (be16_to_cpu(pdu->params_len) < count) {
return;
}
for (i = 0; len > 0; len--, i++) {
if (pdu->params[i + 1] == AVRCP_ATTRIBUTE_ILEGAL ||
pdu->params[i + 1] > AVRCP_ATTRIBUTE_LAST)
continue;
attrs[count++] = pdu->params[i + 1]; /* STACK OVERFLOW WRITE past attrs[4] */
}
avrcp_get_current_player_value(session, attrs, count);
```
#### Historical Root Cause:
Commit `6260f507a5` (Feb 11, 2014) introduced attribute filtering. It initialized `count = 0` to count valid attributes, but left the check `if (params_len < count)` unchanged. Because `count == 0` at the point of check, the bounds check has been dead code for 12 years.
#### Second Stack Overflow:
In `avrcp_get_current_player_value()`:
```c
uint8_t buf[AVRCP_HEADER_LENGTH + AVRCP_ATTRIBUTE_LAST + 1]; /* 12 bytes */
...
memcpy(pdu->params + 1, attrs, count); /* Overwrites stack past 12 bytes with count=255 */
```
#### ASan Trace Output:
```
=================================================================
==540104==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7f8d5e100064 at pc 0x562069cf5afe bp 0x7ffecfa19a80 sp 0x7ffecfa19a78
WRITE of size 1 at 0x7f8d5e100064 thread T0
#0 0x562069cf5afd in avrcp_list_player_attributes_rsp profiles/audio/avrcp.c:155
#1 0x562069cf5de3 in main
```
---
### Finding #2: Heap Out-Of-Bounds Read in `parse_media_element()`
- **File**: `profiles/audio/avrcp.c:2611-2649`
- **Vulnerability Type**: Heap Out-Of-Bounds Read (`READ`)
#### Vulnerable Code:
```c
static void parse_media_element(uint8_t *operands, uint16_t len)
{
uint16_t namelen, namesize;
char name[255];
if (len < 13)
return;
namesize = get_be16(&operands[11]); /* Attacker controlled */
namelen = MIN(namesize, sizeof(name) - 1); /* Capped to 254 for memcpy */
if (namelen > 0)
memcpy(name, &operands[13], namelen); /* HEAP OOB READ if 13 + namelen > len */
count = operands[13 + namesize]; /* HEAP OOB READ if 13 + namesize > len */
if (count > 0)
avrcp_parse_attribute_list(&operands[14 + namesize], count);
}
```
#### Root Cause:
`namesize` is extracted from `operands[11..12]` but is never validated against `len` (the size of the received item buffer). If `13 + namesize > len`, both `memcpy` and `operands[13 + namesize]` perform heap out-of-bounds reads.
#### ASan Trace Output:
```
=================================================================
==540201==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x503000000054 at pc 0x7f88ac8f2936 bp 0x7ffc5e73f930 sp 0x7ffc5e73f0f0
READ of size 254 at 0x503000000054 thread T0
#0 0x7f88ac8f2935 in memcpy
#1 0x558514426760 in parse_media_element profiles/audio/avrcp.c:127
```
---
## Suggested Remediation / Patches
### Patch for Finding #1 (`avrcp_list_player_attributes_rsp`):
```diff
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -2407,8 +2407,11 @@ static void avrcp_list_player_attributes_rsp(struct avrcp *session,
for (i = 0; len > 0; len--, i++) {
if (pdu->params[i + 1] == AVRCP_ATTRIBUTE_ILEGAL ||
pdu->params[i + 1] > AVRCP_ATTRIBUTE_LAST)
continue;
+ if (count >= sizeof(attrs))
+ break;
+
attrs[count++] = pdu->params[i + 1];
}
```
### Patch for Finding #2 (`parse_media_element`):
```diff
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -2628,6 +2628,9 @@ static void parse_media_element(struct avrcp *session,
namesize = get_be16(&operands[11]);
+ if (len < 14 + namesize)
+ return;
+
namelen = MIN(namesize, sizeof(name) - 1);
```