[BlueZ, v2 0/3] avrcp: Fix Out-of-Bounds Read in AVRCP GetFolderItems parsing

[BlueZ, v2 0/3] avrcp: Fix Out-of-Bounds Read in AVRCP GetFolderItems parsing

[Bastien Nocera]

Changes since v1:
- Add missing inttypes.h include to split-off parsing functions

Bastien Nocera (2):
  avrcp: Split off name parsing from parse_*_element()
  unit: Adapt poc_*_oob.c test into a new test

Elman Shahbazov (1):
  avrcp: Fix Out-of-Bounds Read in AVRCP GetFolderItems parsing

 Makefile.am                  |  9 +++++
 Makefile.plugins             |  1 +
 profiles/audio/avrcp-parse.c | 52 ++++++++++++++++++++++++
 profiles/audio/avrcp-parse.h | 19 +++++++++
 profiles/audio/avrcp.c       | 26 +++---------
 unit/test-avrcp-sec.c        | 76 ++++++++++++++++++++++++++++++++++++
 6 files changed, 163 insertions(+), 20 deletions(-)
 create mode 100644 profiles/audio/avrcp-parse.c
 create mode 100644 profiles/audio/avrcp-parse.h
 create mode 100644 unit/test-avrcp-sec.c

-- 
2.55.0

[BlueZ, v2 1/3] avrcp: Split off name parsing from parse_*_element()

[Bastien Nocera]

This will allow us to use the name extraction code in
parse_media_element() and parse_folder_element() separately, such
as in tests.
---
 Makefile.plugins             |  1 +
 profiles/audio/avrcp-parse.c | 47 ++++++++++++++++++++++++++++++++++++
 profiles/audio/avrcp-parse.h | 19 +++++++++++++++
 profiles/audio/avrcp.c       | 26 +++++---------------
 4 files changed, 73 insertions(+), 20 deletions(-)
 create mode 100644 profiles/audio/avrcp-parse.c
 create mode 100644 profiles/audio/avrcp-parse.h

diff --git a/Makefile.plugins b/Makefile.plugins
index ac667beda847..a505fcd6691f 100644
--- a/Makefile.plugins
+++ b/Makefile.plugins
@@ -37,6 +37,7 @@ builtin_modules += avrcp
 builtin_sources += profiles/audio/control.h profiles/audio/control.c \
 			profiles/audio/avctp.h profiles/audio/avctp.c \
 			profiles/audio/avrcp.h profiles/audio/avrcp.c \
+			profiles/audio/avrcp-parse.h profiles/audio/avrcp-parse.c \
 			profiles/audio/avrcp-player.c
 endif
 
diff --git a/profiles/audio/avrcp-parse.c b/profiles/audio/avrcp-parse.c
new file mode 100644
index 000000000000..d3d0a070a4da
--- /dev/null
+++ b/profiles/audio/avrcp-parse.c
@@ -0,0 +1,47 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2026  Red Hat Inc.
+ *
+ *
+ */
+
+#include "avrcp-parse.h"
+#include "src/shared/util.h"
+
+gboolean parse_media_element_name(uint8_t *operands, uint16_t len,
+					 char *name, uint16_t *namesize)
+{
+	uint16_t namelen;
+
+	if (len < 13)
+		return FALSE;
+
+	memset(name, 0, NAME_MAX_LEN);
+	*namesize = get_be16(&operands[11]);
+	namelen = MIN(*namesize, NAME_MAX_LEN - 1);
+	if (namelen > 0) {
+		memcpy(name, &operands[13], namelen);
+		strtoutf8(name, namelen);
+	}
+
+	return TRUE;
+}
+
+gboolean parse_media_folder_name(uint8_t *operands, uint16_t len,
+					char *name)
+{
+	uint16_t namelen;
+
+	if (len < 12)
+		return FALSE;
+
+	memset(name, 0, NAME_MAX_LEN);
+	namelen = MIN(get_be16(&operands[12]), NAME_MAX_LEN - 1);
+	if (namelen > 0)
+		memcpy(name, &operands[14], namelen);
+
+	return TRUE;
+}
diff --git a/profiles/audio/avrcp-parse.h b/profiles/audio/avrcp-parse.h
new file mode 100644
index 000000000000..f7a33c854442
--- /dev/null
+++ b/profiles/audio/avrcp-parse.h
@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2026  Red Hat Inc.
+ *
+ *
+ */
+
+#include <glib.h>
+#include <inttypes.h>
+
+#define NAME_MAX_LEN 255
+
+gboolean parse_media_element_name(uint8_t *operands, uint16_t len,
+					 char *name, uint16_t *namesize);
+gboolean parse_media_folder_name(uint8_t *operands, uint16_t len,
+					char *name);
diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index 2194a913580f..af3c72174764 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -52,6 +52,7 @@
 
 #include "avctp.h"
 #include "avrcp.h"
+#include "avrcp-parse.h"
 #include "control.h"
 #include "media.h"
 #include "player.h"
@@ -2614,24 +2615,15 @@ static struct media_item *parse_media_element(struct avrcp *session,
 	struct avrcp_player *player;
 	struct media_player *mp;
 	struct media_item *item;
-	uint16_t namelen, namesize;
-	char name[255];
+	uint16_t namesize;
+	char name[NAME_MAX_LEN];
 	uint64_t uid;
 	uint8_t count;
 
-	if (len < 13)
+	if (!parse_media_element_name(operands, len, name, &namesize))
 		return NULL;
 
 	uid = get_be64(&operands[0]);
-
-	memset(name, 0, sizeof(name));
-	namesize = get_be16(&operands[11]);
-	namelen = MIN(namesize, sizeof(name) - 1);
-	if (namelen > 0) {
-		memcpy(name, &operands[13], namelen);
-		strtoutf8(name, namelen);
-	}
-
 	count = operands[13 + namesize];
 
 	player = session->controller->player;
@@ -2655,24 +2647,18 @@ static struct media_item *parse_media_folder(struct avrcp *session,
 	struct avrcp_player *player = session->controller->player;
 	struct media_player *mp = player->user_data;
 	struct media_item *item;
-	uint16_t namelen;
-	char name[255];
+	char name[NAME_MAX_LEN];
 	uint64_t uid;
 	uint8_t type;
 	uint8_t playable;
 
-	if (len < 12)
+	if (!parse_media_folder_name(operands, len, name))
 		return NULL;
 
 	uid = get_be64(&operands[0]);
 	type = operands[8];
 	playable = operands[9];
 
-	memset(name, 0, sizeof(name));
-	namelen = MIN(get_be16(&operands[12]), sizeof(name) - 1);
-	if (namelen > 0)
-		memcpy(name, &operands[14], namelen);
-
 	item = media_player_create_folder(mp, name, type, uid);
 	if (!item)
 		return NULL;
-- 
2.55.0

[BlueZ, v2 2/3] unit: Adapt poc_*_oob.c test into a new test

[Bastien Nocera]

Adapt poc_avrcp_oob.c and poc_folder_oob.c into unit tests.

Co-authored-by: Elman Shahbazov <shahbazovelman97@gmail.com>
---
 Makefile.am           |  9 +++++
 unit/test-avrcp-sec.c | 76 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 85 insertions(+)
 create mode 100644 unit/test-avrcp-sec.c

diff --git a/Makefile.am b/Makefile.am
index 19c468d3a504..e3baa4155c1f 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -660,6 +660,15 @@ unit_test_avrcp_SOURCES = unit/test-avrcp.c \
 unit_test_avrcp_LDADD = lib/libbluetooth-internal.la \
 				src/libshared-glib.la $(GLIB_LIBS)
 
+unit_tests += unit/test-avrcp-sec
+
+unit_test_avrcp_sec_SOURCES = unit/test-avrcp-sec.c \
+			profiles/audio/avrcp-parse.c \
+			profiles/audio/avrcp-parse.h \
+			src/log.h src/log.c
+unit_test_avrcp_sec_LDADD = lib/libbluetooth-internal.la \
+				src/libshared-glib.la $(GLIB_LIBS)
+
 unit_tests += unit/test-hfp
 
 unit_test_hfp_SOURCES = unit/test-hfp.c
diff --git a/unit/test-avrcp-sec.c b/unit/test-avrcp-sec.c
new file mode 100644
index 000000000000..a100b2d68e35
--- /dev/null
+++ b/unit/test-avrcp-sec.c
@@ -0,0 +1,76 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2026  Red Hat Inc.
+ *
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <glib.h>
+
+#include "src/shared/util.h"
+#include "src/shared/tester.h"
+#include "src/log.h"
+
+#include "profiles/audio/avrcp-parse.h"
+
+static void avrcp_element_name_oob(gconstpointer data)
+{
+	char name[255];
+	uint16_t namesize;
+	gboolean ret;
+
+	/* Crafting a malicious payload.
+	 * Actual packet length (len) = 14 bytes */
+	uint8_t malicious_packet[14] = {0};
+
+	/* Specify namesize = 1000 (0x03E8 in Big Endian) at offset 11 */
+	malicious_packet[11] = 0x03;
+	malicious_packet[12] = 0xE8;
+
+	/* Launching the PoC. We transmit a 14-byte packet, but namesize=1000... */
+	ret = parse_media_element_name(malicious_packet, sizeof(malicious_packet),
+				       name, &namesize);
+	if (ret)
+		tester_test_passed();
+	else
+		tester_test_failed();
+}
+
+static void avrcp_folder_name_oob(gconstpointer data)
+{
+	char name[255];
+	gboolean ret;
+
+	/* Crafting a malicious payload.
+	 * Actual packet length (len) = 14 bytes */
+	uint8_t malicious_packet[14] = {0};
+
+	/* Specify namesize = 1000 (0x03E8 in Big Endian) at offset 12 */
+	malicious_packet[12] = 0x03;
+	malicious_packet[13] = 0xE8;
+
+	/* Launching the PoC. We transmit a 14-byte packet, but namesize=1000... */
+	ret = parse_media_folder_name(malicious_packet, sizeof(malicious_packet),
+				      name);
+	if (ret)
+		tester_test_passed();
+	else
+		tester_test_failed();
+}
+
+int main(int argc, char *argv[])
+{
+	tester_init(&argc, &argv);
+
+	tester_add("/avrcp-element-name-oob", NULL, NULL, avrcp_element_name_oob, NULL);
+	tester_add("/avrcp-folder-name-oob", NULL, NULL, avrcp_folder_name_oob, NULL);
+
+	return tester_run();
+}
-- 
2.55.0

[BlueZ, v2 3/3] avrcp: Fix Out-of-Bounds Read in AVRCP GetFolderItems parsing

[Bastien Nocera]

From: Elman Shahbazov <shahbazovelman97@gmail.com>

Co-Authored-by: Bastien Nocera <hadess@hadess.net>
---
 profiles/audio/avrcp-parse.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/profiles/audio/avrcp-parse.c b/profiles/audio/avrcp-parse.c
index d3d0a070a4da..251580fd9c55 100644
--- a/profiles/audio/avrcp-parse.c
+++ b/profiles/audio/avrcp-parse.c
@@ -20,8 +20,12 @@ gboolean parse_media_element_name(uint8_t *operands, uint16_t len,
 		return FALSE;
 
 	memset(name, 0, NAME_MAX_LEN);
-	*namesize = get_be16(&operands[11]);
+	*namesize = MIN(get_be16(&operands[11]), len - 13);
 	namelen = MIN(*namesize, NAME_MAX_LEN - 1);
+
+	if (len < 13 + *namesize)
+		return FALSE;
+
 	if (namelen > 0) {
 		memcpy(name, &operands[13], namelen);
 		strtoutf8(name, namelen);
@@ -39,7 +43,8 @@ gboolean parse_media_folder_name(uint8_t *operands, uint16_t len,
 		return FALSE;
 
 	memset(name, 0, NAME_MAX_LEN);
-	namelen = MIN(get_be16(&operands[12]), NAME_MAX_LEN - 1);
+	namelen = MIN(get_be16(&operands[12]), len - 14);
+	namelen = MIN(namelen, NAME_MAX_LEN - 1);
 	if (namelen > 0)
 		memcpy(name, &operands[14], namelen);
 
-- 
2.55.0

RE: avrcp: Fix Out-of-Bounds Read in AVRCP GetFolderItems parsing

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 4212 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=1140181

---Test result---

Test Summary:
CheckPatch                    FAIL      1.01 seconds
GitLint                       PASS      0.65 seconds
BuildEll                      PASS      21.33 seconds
BluezMake                     PASS      551.90 seconds
MakeCheck                     PASS      19.02 seconds
MakeDistcheck                 PASS      159.31 seconds
CheckValgrind                 PASS      227.71 seconds
CheckSmatch                   PASS      309.84 seconds
bluezmakeextell               PASS      98.23 seconds
IncrementalBuild              PASS      563.59 seconds
ScanBuild                     PASS      945.33 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
[BlueZ,v2,2/3] unit: Adapt poc_*_oob.c test into a new test
WARNING:BAD_SIGN_OFF: Non-standard signature: Co-authored-by:
#56: 
Co-authored-by: Elman Shahbazov <shahbazovelman97@gmail.com>

WARNING:BLOCK_COMMENT_STYLE: Block comments use a trailing */ on a separate line
#118: FILE: unit/test-avrcp-sec.c:30:
+	 * Actual packet length (len) = 14 bytes */

WARNING:LONG_LINE_COMMENT: line length of 83 exceeds 80 columns
#125: FILE: unit/test-avrcp-sec.c:37:
+	/* Launching the PoC. We transmit a 14-byte packet, but namesize=1000... */

WARNING:LONG_LINE: line length of 82 exceeds 80 columns
#126: FILE: unit/test-avrcp-sec.c:38:
+	ret = parse_media_element_name(malicious_packet, sizeof(malicious_packet),

WARNING:BLOCK_COMMENT_STYLE: Block comments use a trailing */ on a separate line
#140: FILE: unit/test-avrcp-sec.c:52:
+	 * Actual packet length (len) = 14 bytes */

WARNING:LONG_LINE_COMMENT: line length of 83 exceeds 80 columns
#147: FILE: unit/test-avrcp-sec.c:59:
+	/* Launching the PoC. We transmit a 14-byte packet, but namesize=1000... */

WARNING:LONG_LINE: line length of 81 exceeds 80 columns
#148: FILE: unit/test-avrcp-sec.c:60:
+	ret = parse_media_folder_name(malicious_packet, sizeof(malicious_packet),

WARNING:LONG_LINE: line length of 88 exceeds 80 columns
#160: FILE: unit/test-avrcp-sec.c:72:
+	tester_add("/avrcp-element-name-oob", NULL, NULL, avrcp_element_name_oob, NULL);

WARNING:LONG_LINE: line length of 86 exceeds 80 columns
#161: FILE: unit/test-avrcp-sec.c:73:
+	tester_add("/avrcp-folder-name-oob", NULL, NULL, avrcp_folder_name_oob, NULL);

/github/workspace/src/patch/14730317.patch total: 0 errors, 9 warnings, 91 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

/github/workspace/src/patch/14730317.patch has style problems, please review.

NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.


[BlueZ,v2,3/3] avrcp: Fix Out-of-Bounds Read in AVRCP GetFolderItems parsing
WARNING:BAD_SIGN_OFF: Non-standard signature: Co-Authored-by:
#57: 
Co-Authored-by: Bastien Nocera <hadess@hadess.net>

WARNING:BAD_SIGN_OFF: 'Co-authored-by:' is the preferred signature form
#57: 
Co-Authored-by: Bastien Nocera <hadess@hadess.net>

/github/workspace/src/patch/14730318.patch total: 0 errors, 2 warnings, 22 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

/github/workspace/src/patch/14730318.patch has style problems, please review.

NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.




https://github.com/bluez/bluez/pull/2376

---
Regards,
Linux Bluetooth

Re: [BlueZ, v2 2/3] unit: Adapt poc_*_oob.c test into a new test

[Luiz Augusto von Dentz]

Hi Bastien,

On Tue, Aug 4, 2026 at 10:31 AM Bastien Nocera <hadess@hadess.net> wrote:
>
> Adapt poc_avrcp_oob.c and poc_folder_oob.c into unit tests.
>
> Co-authored-by: Elman Shahbazov <shahbazovelman97@gmail.com>
> ---
>  Makefile.am           |  9 +++++
>  unit/test-avrcp-sec.c | 76 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 85 insertions(+)
>  create mode 100644 unit/test-avrcp-sec.c
>
> diff --git a/Makefile.am b/Makefile.am
> index 19c468d3a504..e3baa4155c1f 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -660,6 +660,15 @@ unit_test_avrcp_SOURCES = unit/test-avrcp.c \
>  unit_test_avrcp_LDADD = lib/libbluetooth-internal.la \
>                                 src/libshared-glib.la $(GLIB_LIBS)
>
> +unit_tests += unit/test-avrcp-sec
> +
> +unit_test_avrcp_sec_SOURCES = unit/test-avrcp-sec.c \
> +                       profiles/audio/avrcp-parse.c \
> +                       profiles/audio/avrcp-parse.h \
> +                       src/log.h src/log.c
> +unit_test_avrcp_sec_LDADD = lib/libbluetooth-internal.la \
> +                               src/libshared-glib.la $(GLIB_LIBS)

Any reason why this couldn't live inside test-avrcp.c?

>  unit_tests += unit/test-hfp
>
>  unit_test_hfp_SOURCES = unit/test-hfp.c
> diff --git a/unit/test-avrcp-sec.c b/unit/test-avrcp-sec.c
> new file mode 100644
> index 000000000000..a100b2d68e35
> --- /dev/null
> +++ b/unit/test-avrcp-sec.c
> @@ -0,0 +1,76 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + *
> + *  BlueZ - Bluetooth protocol stack for Linux
> + *
> + *  Copyright (C) 2026  Red Hat Inc.
> + *
> + *
> + */
> +
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
> +#include <glib.h>
> +
> +#include "src/shared/util.h"
> +#include "src/shared/tester.h"
> +#include "src/log.h"
> +
> +#include "profiles/audio/avrcp-parse.h"
> +
> +static void avrcp_element_name_oob(gconstpointer data)
> +{
> +       char name[255];
> +       uint16_t namesize;
> +       gboolean ret;
> +
> +       /* Crafting a malicious payload.
> +        * Actual packet length (len) = 14 bytes */
> +       uint8_t malicious_packet[14] = {0};
> +
> +       /* Specify namesize = 1000 (0x03E8 in Big Endian) at offset 11 */
> +       malicious_packet[11] = 0x03;
> +       malicious_packet[12] = 0xE8;
> +
> +       /* Launching the PoC. We transmit a 14-byte packet, but namesize=1000... */
> +       ret = parse_media_element_name(malicious_packet, sizeof(malicious_packet),
> +                                      name, &namesize);
> +       if (ret)
> +               tester_test_passed();
> +       else
> +               tester_test_failed();
> +}
> +
> +static void avrcp_folder_name_oob(gconstpointer data)
> +{
> +       char name[255];
> +       gboolean ret;
> +
> +       /* Crafting a malicious payload.
> +        * Actual packet length (len) = 14 bytes */
> +       uint8_t malicious_packet[14] = {0};
> +
> +       /* Specify namesize = 1000 (0x03E8 in Big Endian) at offset 12 */
> +       malicious_packet[12] = 0x03;
> +       malicious_packet[13] = 0xE8;
> +
> +       /* Launching the PoC. We transmit a 14-byte packet, but namesize=1000... */
> +       ret = parse_media_folder_name(malicious_packet, sizeof(malicious_packet),
> +                                     name);
> +       if (ret)
> +               tester_test_passed();
> +       else
> +               tester_test_failed();
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +       tester_init(&argc, &argv);
> +
> +       tester_add("/avrcp-element-name-oob", NULL, NULL, avrcp_element_name_oob, NULL);
> +       tester_add("/avrcp-folder-name-oob", NULL, NULL, avrcp_folder_name_oob, NULL);
> +
> +       return tester_run();
> +}
> --
> 2.55.0
>
>


-- 
Luiz Augusto von Dentz

Re: [BlueZ, v2 2/3] unit: Adapt poc_*_oob.c test into a new test

[Bastien Nocera]

On Tue, 2026-08-04 at 12:46 -0400, Luiz Augusto von Dentz wrote:
> Hi Bastien,
> 
> On Tue, Aug 4, 2026 at 10:31 AM Bastien Nocera <hadess@hadess.net>
> wrote:
> > 
> > Adapt poc_avrcp_oob.c and poc_folder_oob.c into unit tests.
> > 
> > Co-authored-by: Elman Shahbazov <shahbazovelman97@gmail.com>
> > ---
> >  Makefile.am           |  9 +++++
> >  unit/test-avrcp-sec.c | 76
> > +++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 85 insertions(+)
> >  create mode 100644 unit/test-avrcp-sec.c
> > 
> > diff --git a/Makefile.am b/Makefile.am
> > index 19c468d3a504..e3baa4155c1f 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -660,6 +660,15 @@ unit_test_avrcp_SOURCES = unit/test-avrcp.c \
> >  unit_test_avrcp_LDADD = lib/libbluetooth-internal.la \
> >                                 src/libshared-glib.la $(GLIB_LIBS)
> > 
> > +unit_tests += unit/test-avrcp-sec
> > +
> > +unit_test_avrcp_sec_SOURCES = unit/test-avrcp-sec.c \
> > +                       profiles/audio/avrcp-parse.c \
> > +                       profiles/audio/avrcp-parse.h \
> > +                       src/log.h src/log.c
> > +unit_test_avrcp_sec_LDADD = lib/libbluetooth-internal.la \
> > +                               src/libshared-glib.la $(GLIB_LIBS)
> 
> Any reason why this couldn't live inside test-avrcp.c?

Because most of the code in test-avrcp.c is mocked in unit/avrcp-
lib.[ch] and conflicts with code from profiles/audio/avrcp.c. I really
did try...

I can change the name of the test if needed.

> 
> >  unit_tests += unit/test-hfp
> > 
> >  unit_test_hfp_SOURCES = unit/test-hfp.c
> > diff --git a/unit/test-avrcp-sec.c b/unit/test-avrcp-sec.c
> > new file mode 100644
> > index 000000000000..a100b2d68e35
> > --- /dev/null
> > +++ b/unit/test-avrcp-sec.c
> > @@ -0,0 +1,76 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + *
> > + *  BlueZ - Bluetooth protocol stack for Linux
> > + *
> > + *  Copyright (C) 2026  Red Hat Inc.
> > + *
> > + *
> > + */
> > +
> > +#ifdef HAVE_CONFIG_H
> > +#include <config.h>
> > +#endif
> > +
> > +#include <glib.h>
> > +
> > +#include "src/shared/util.h"
> > +#include "src/shared/tester.h"
> > +#include "src/log.h"
> > +
> > +#include "profiles/audio/avrcp-parse.h"
> > +
> > +static void avrcp_element_name_oob(gconstpointer data)
> > +{
> > +       char name[255];
> > +       uint16_t namesize;
> > +       gboolean ret;
> > +
> > +       /* Crafting a malicious payload.
> > +        * Actual packet length (len) = 14 bytes */
> > +       uint8_t malicious_packet[14] = {0};
> > +
> > +       /* Specify namesize = 1000 (0x03E8 in Big Endian) at offset
> > 11 */
> > +       malicious_packet[11] = 0x03;
> > +       malicious_packet[12] = 0xE8;
> > +
> > +       /* Launching the PoC. We transmit a 14-byte packet, but
> > namesize=1000... */
> > +       ret = parse_media_element_name(malicious_packet,
> > sizeof(malicious_packet),
> > +                                      name, &namesize);
> > +       if (ret)
> > +               tester_test_passed();
> > +       else
> > +               tester_test_failed();
> > +}
> > +
> > +static void avrcp_folder_name_oob(gconstpointer data)
> > +{
> > +       char name[255];
> > +       gboolean ret;
> > +
> > +       /* Crafting a malicious payload.
> > +        * Actual packet length (len) = 14 bytes */
> > +       uint8_t malicious_packet[14] = {0};
> > +
> > +       /* Specify namesize = 1000 (0x03E8 in Big Endian) at offset
> > 12 */
> > +       malicious_packet[12] = 0x03;
> > +       malicious_packet[13] = 0xE8;
> > +
> > +       /* Launching the PoC. We transmit a 14-byte packet, but
> > namesize=1000... */
> > +       ret = parse_media_folder_name(malicious_packet,
> > sizeof(malicious_packet),
> > +                                     name);
> > +       if (ret)
> > +               tester_test_passed();
> > +       else
> > +               tester_test_failed();
> > +}
> > +
> > +int main(int argc, char *argv[])
> > +{
> > +       tester_init(&argc, &argv);
> > +
> > +       tester_add("/avrcp-element-name-oob", NULL, NULL,
> > avrcp_element_name_oob, NULL);
> > +       tester_add("/avrcp-folder-name-oob", NULL, NULL,
> > avrcp_folder_name_oob, NULL);
> > +
> > +       return tester_run();
> > +}
> > --
> > 2.55.0
> > 
> > 
> 

Re: [BlueZ, v2 2/3] unit: Adapt poc_*_oob.c test into a new test

[Luiz Augusto von Dentz]

Hi Bastien,

On Wed, Aug 5, 2026 at 3:59 AM Bastien Nocera <hadess@hadess.net> wrote:
>
> On Tue, 2026-08-04 at 12:46 -0400, Luiz Augusto von Dentz wrote:
> > Hi Bastien,
> >
> > On Tue, Aug 4, 2026 at 10:31 AM Bastien Nocera <hadess@hadess.net>
> > wrote:
> > >
> > > Adapt poc_avrcp_oob.c and poc_folder_oob.c into unit tests.
> > >
> > > Co-authored-by: Elman Shahbazov <shahbazovelman97@gmail.com>
> > > ---
> > >  Makefile.am           |  9 +++++
> > >  unit/test-avrcp-sec.c | 76
> > > +++++++++++++++++++++++++++++++++++++++++++
> > >  2 files changed, 85 insertions(+)
> > >  create mode 100644 unit/test-avrcp-sec.c
> > >
> > > diff --git a/Makefile.am b/Makefile.am
> > > index 19c468d3a504..e3baa4155c1f 100644
> > > --- a/Makefile.am
> > > +++ b/Makefile.am
> > > @@ -660,6 +660,15 @@ unit_test_avrcp_SOURCES = unit/test-avrcp.c \
> > >  unit_test_avrcp_LDADD = lib/libbluetooth-internal.la \
> > >                                 src/libshared-glib.la $(GLIB_LIBS)
> > >
> > > +unit_tests += unit/test-avrcp-sec
> > > +
> > > +unit_test_avrcp_sec_SOURCES = unit/test-avrcp-sec.c \
> > > +                       profiles/audio/avrcp-parse.c \
> > > +                       profiles/audio/avrcp-parse.h \
> > > +                       src/log.h src/log.c
> > > +unit_test_avrcp_sec_LDADD = lib/libbluetooth-internal.la \
> > > +                               src/libshared-glib.la $(GLIB_LIBS)
> >
> > Any reason why this couldn't live inside test-avrcp.c?
>
> Because most of the code in test-avrcp.c is mocked in unit/avrcp-
> lib.[ch] and conflicts with code from profiles/audio/avrcp.c. I really
> did try...

Ok, well then perhaps we should start testing what we really use in
the avrcp plugin rather than the removed Android code, or we could
have a bigger task: converting this type of code to src/shared/avrcp.c
(e.g. bt_avrcp) so it can be unit tested.

> I can change the name of the test if needed.

I'm not concerned about the name, I'm concerned about the number of
unit test files which could otherwise be in a single file instead of
duplicating a lot of context.

> >
> > >  unit_tests += unit/test-hfp
> > >
> > >  unit_test_hfp_SOURCES = unit/test-hfp.c
> > > diff --git a/unit/test-avrcp-sec.c b/unit/test-avrcp-sec.c
> > > new file mode 100644
> > > index 000000000000..a100b2d68e35
> > > --- /dev/null
> > > +++ b/unit/test-avrcp-sec.c
> > > @@ -0,0 +1,76 @@
> > > +// SPDX-License-Identifier: GPL-2.0-or-later
> > > +/*
> > > + *
> > > + *  BlueZ - Bluetooth protocol stack for Linux
> > > + *
> > > + *  Copyright (C) 2026  Red Hat Inc.
> > > + *
> > > + *
> > > + */
> > > +
> > > +#ifdef HAVE_CONFIG_H
> > > +#include <config.h>
> > > +#endif
> > > +
> > > +#include <glib.h>
> > > +
> > > +#include "src/shared/util.h"
> > > +#include "src/shared/tester.h"
> > > +#include "src/log.h"
> > > +
> > > +#include "profiles/audio/avrcp-parse.h"
> > > +
> > > +static void avrcp_element_name_oob(gconstpointer data)
> > > +{
> > > +       char name[255];
> > > +       uint16_t namesize;
> > > +       gboolean ret;
> > > +
> > > +       /* Crafting a malicious payload.
> > > +        * Actual packet length (len) = 14 bytes */
> > > +       uint8_t malicious_packet[14] = {0};
> > > +
> > > +       /* Specify namesize = 1000 (0x03E8 in Big Endian) at offset
> > > 11 */
> > > +       malicious_packet[11] = 0x03;
> > > +       malicious_packet[12] = 0xE8;
> > > +
> > > +       /* Launching the PoC. We transmit a 14-byte packet, but
> > > namesize=1000... */
> > > +       ret = parse_media_element_name(malicious_packet,
> > > sizeof(malicious_packet),
> > > +                                      name, &namesize);
> > > +       if (ret)
> > > +               tester_test_passed();
> > > +       else
> > > +               tester_test_failed();
> > > +}
> > > +
> > > +static void avrcp_folder_name_oob(gconstpointer data)
> > > +{
> > > +       char name[255];
> > > +       gboolean ret;
> > > +
> > > +       /* Crafting a malicious payload.
> > > +        * Actual packet length (len) = 14 bytes */
> > > +       uint8_t malicious_packet[14] = {0};
> > > +
> > > +       /* Specify namesize = 1000 (0x03E8 in Big Endian) at offset
> > > 12 */
> > > +       malicious_packet[12] = 0x03;
> > > +       malicious_packet[13] = 0xE8;
> > > +
> > > +       /* Launching the PoC. We transmit a 14-byte packet, but
> > > namesize=1000... */
> > > +       ret = parse_media_folder_name(malicious_packet,
> > > sizeof(malicious_packet),
> > > +                                     name);
> > > +       if (ret)
> > > +               tester_test_passed();
> > > +       else
> > > +               tester_test_failed();
> > > +}
> > > +
> > > +int main(int argc, char *argv[])
> > > +{
> > > +       tester_init(&argc, &argv);
> > > +
> > > +       tester_add("/avrcp-element-name-oob", NULL, NULL,
> > > avrcp_element_name_oob, NULL);
> > > +       tester_add("/avrcp-folder-name-oob", NULL, NULL,
> > > avrcp_folder_name_oob, NULL);
> > > +
> > > +       return tester_run();
> > > +}
> > > --
> > > 2.55.0
> > >
> > >
> >



-- 
Luiz Augusto von Dentz

Re: [BlueZ, v2 1/3] avrcp: Split off name parsing from parse_*_element()

[Luiz Augusto von Dentz]

Hi Bastien,

On Tue, Aug 4, 2026 at 10:31 AM Bastien Nocera <hadess@hadess.net> wrote:
>
> This will allow us to use the name extraction code in
> parse_media_element() and parse_folder_element() separately, such
> as in tests.
> ---
>  Makefile.plugins             |  1 +
>  profiles/audio/avrcp-parse.c | 47 ++++++++++++++++++++++++++++++++++++
>  profiles/audio/avrcp-parse.h | 19 +++++++++++++++
>  profiles/audio/avrcp.c       | 26 +++++---------------
>  4 files changed, 73 insertions(+), 20 deletions(-)
>  create mode 100644 profiles/audio/avrcp-parse.c
>  create mode 100644 profiles/audio/avrcp-parse.h
>
> diff --git a/Makefile.plugins b/Makefile.plugins
> index ac667beda847..a505fcd6691f 100644
> --- a/Makefile.plugins
> +++ b/Makefile.plugins
> @@ -37,6 +37,7 @@ builtin_modules += avrcp
>  builtin_sources += profiles/audio/control.h profiles/audio/control.c \
>                         profiles/audio/avctp.h profiles/audio/avctp.c \
>                         profiles/audio/avrcp.h profiles/audio/avrcp.c \
> +                       profiles/audio/avrcp-parse.h profiles/audio/avrcp-parse.c \
>                         profiles/audio/avrcp-player.c
>  endif
>
> diff --git a/profiles/audio/avrcp-parse.c b/profiles/audio/avrcp-parse.c
> new file mode 100644
> index 000000000000..d3d0a070a4da
> --- /dev/null
> +++ b/profiles/audio/avrcp-parse.c
> @@ -0,0 +1,47 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + *
> + *  BlueZ - Bluetooth protocol stack for Linux
> + *
> + *  Copyright (C) 2026  Red Hat Inc.
> + *
> + *
> + */
> +
> +#include "avrcp-parse.h"
> +#include "src/shared/util.h"
> +
> +gboolean parse_media_element_name(uint8_t *operands, uint16_t len,
> +                                        char *name, uint16_t *namesize)
> +{
> +       uint16_t namelen;
> +
> +       if (len < 13)
> +               return FALSE;
> +
> +       memset(name, 0, NAME_MAX_LEN);
> +       *namesize = get_be16(&operands[11]);
> +       namelen = MIN(*namesize, NAME_MAX_LEN - 1);
> +       if (namelen > 0) {
> +               memcpy(name, &operands[13], namelen);
> +               strtoutf8(name, namelen);
> +       }
> +
> +       return TRUE;
> +}
> +
> +gboolean parse_media_folder_name(uint8_t *operands, uint16_t len,
> +                                       char *name)
> +{
> +       uint16_t namelen;
> +
> +       if (len < 12)
> +               return FALSE;
> +
> +       memset(name, 0, NAME_MAX_LEN);
> +       namelen = MIN(get_be16(&operands[12]), NAME_MAX_LEN - 1);
> +       if (namelen > 0)
> +               memcpy(name, &operands[14], namelen);
> +
> +       return TRUE;
> +}

Rather than creating yet another file how about hosting this under
shared/util.h directly? It already depends on it anyway, we could got
with something like strntoutf8 or a similar function that checks the
length, etc, actually be maybe better to do it under
util_iov_pull_utf8(iov, len, str, str_max_len) so we can load the pdu
into the iov then use iov_pull_mem, etc, to verify that we have enough
bytes in a generic manner.

> diff --git a/profiles/audio/avrcp-parse.h b/profiles/audio/avrcp-parse.h
> new file mode 100644
> index 000000000000..f7a33c854442
> --- /dev/null
> +++ b/profiles/audio/avrcp-parse.h
> @@ -0,0 +1,19 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + *
> + *  BlueZ - Bluetooth protocol stack for Linux
> + *
> + *  Copyright (C) 2026  Red Hat Inc.
> + *
> + *
> + */
> +
> +#include <glib.h>
> +#include <inttypes.h>
> +
> +#define NAME_MAX_LEN 255
> +
> +gboolean parse_media_element_name(uint8_t *operands, uint16_t len,
> +                                        char *name, uint16_t *namesize);
> +gboolean parse_media_folder_name(uint8_t *operands, uint16_t len,
> +                                       char *name);
> diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
> index 2194a913580f..af3c72174764 100644
> --- a/profiles/audio/avrcp.c
> +++ b/profiles/audio/avrcp.c
> @@ -52,6 +52,7 @@
>
>  #include "avctp.h"
>  #include "avrcp.h"
> +#include "avrcp-parse.h"
>  #include "control.h"
>  #include "media.h"
>  #include "player.h"
> @@ -2614,24 +2615,15 @@ static struct media_item *parse_media_element(struct avrcp *session,
>         struct avrcp_player *player;
>         struct media_player *mp;
>         struct media_item *item;
> -       uint16_t namelen, namesize;
> -       char name[255];
> +       uint16_t namesize;
> +       char name[NAME_MAX_LEN];
>         uint64_t uid;
>         uint8_t count;
>
> -       if (len < 13)
> +       if (!parse_media_element_name(operands, len, name, &namesize))
>                 return NULL;
>
>         uid = get_be64(&operands[0]);
> -
> -       memset(name, 0, sizeof(name));
> -       namesize = get_be16(&operands[11]);
> -       namelen = MIN(namesize, sizeof(name) - 1);
> -       if (namelen > 0) {
> -               memcpy(name, &operands[13], namelen);
> -               strtoutf8(name, namelen);
> -       }
> -
>         count = operands[13 + namesize];
>
>         player = session->controller->player;
> @@ -2655,24 +2647,18 @@ static struct media_item *parse_media_folder(struct avrcp *session,
>         struct avrcp_player *player = session->controller->player;
>         struct media_player *mp = player->user_data;
>         struct media_item *item;
> -       uint16_t namelen;
> -       char name[255];
> +       char name[NAME_MAX_LEN];
>         uint64_t uid;
>         uint8_t type;
>         uint8_t playable;
>
> -       if (len < 12)
> +       if (!parse_media_folder_name(operands, len, name))
>                 return NULL;
>
>         uid = get_be64(&operands[0]);
>         type = operands[8];
>         playable = operands[9];
>
> -       memset(name, 0, sizeof(name));
> -       namelen = MIN(get_be16(&operands[12]), sizeof(name) - 1);
> -       if (namelen > 0)
> -               memcpy(name, &operands[14], namelen);
> -
>         item = media_player_create_folder(mp, name, type, uid);
>         if (!item)
>                 return NULL;
> --
> 2.55.0
>
>


-- 
Luiz Augusto von Dentz

Re: [BlueZ, v2 1/3] avrcp: Split off name parsing from parse_*_element()

[Bastien Nocera]

On Wed, 2026-08-05 at 13:12 -0400, Luiz Augusto von Dentz wrote:
> Hi Bastien,
> 
> On Tue, Aug 4, 2026 at 10:31 AM Bastien Nocera <hadess@hadess.net>
> wrote:
> > 
> > This will allow us to use the name extraction code in
> > parse_media_element() and parse_folder_element() separately, such
> > as in tests.
> > ---
> >  Makefile.plugins             |  1 +
> >  profiles/audio/avrcp-parse.c | 47
> > ++++++++++++++++++++++++++++++++++++
> >  profiles/audio/avrcp-parse.h | 19 +++++++++++++++
> >  profiles/audio/avrcp.c       | 26 +++++---------------
> >  4 files changed, 73 insertions(+), 20 deletions(-)
> >  create mode 100644 profiles/audio/avrcp-parse.c
> >  create mode 100644 profiles/audio/avrcp-parse.h
> > 
> > diff --git a/Makefile.plugins b/Makefile.plugins
> > index ac667beda847..a505fcd6691f 100644
> > --- a/Makefile.plugins
> > +++ b/Makefile.plugins
> > @@ -37,6 +37,7 @@ builtin_modules += avrcp
> >  builtin_sources += profiles/audio/control.h
> > profiles/audio/control.c \
> >                         profiles/audio/avctp.h
> > profiles/audio/avctp.c \
> >                         profiles/audio/avrcp.h
> > profiles/audio/avrcp.c \
> > +                       profiles/audio/avrcp-parse.h
> > profiles/audio/avrcp-parse.c \
> >                         profiles/audio/avrcp-player.c
> >  endif
> > 
> > diff --git a/profiles/audio/avrcp-parse.c b/profiles/audio/avrcp-
> > parse.c
> > new file mode 100644
> > index 000000000000..d3d0a070a4da
> > --- /dev/null
> > +++ b/profiles/audio/avrcp-parse.c
> > @@ -0,0 +1,47 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + *
> > + *  BlueZ - Bluetooth protocol stack for Linux
> > + *
> > + *  Copyright (C) 2026  Red Hat Inc.
> > + *
> > + *
> > + */
> > +
> > +#include "avrcp-parse.h"
> > +#include "src/shared/util.h"
> > +
> > +gboolean parse_media_element_name(uint8_t *operands, uint16_t len,
> > +                                        char *name, uint16_t
> > *namesize)
> > +{
> > +       uint16_t namelen;
> > +
> > +       if (len < 13)
> > +               return FALSE;
> > +
> > +       memset(name, 0, NAME_MAX_LEN);
> > +       *namesize = get_be16(&operands[11]);
> > +       namelen = MIN(*namesize, NAME_MAX_LEN - 1);
> > +       if (namelen > 0) {
> > +               memcpy(name, &operands[13], namelen);
> > +               strtoutf8(name, namelen);
> > +       }
> > +
> > +       return TRUE;
> > +}
> > +
> > +gboolean parse_media_folder_name(uint8_t *operands, uint16_t len,
> > +                                       char *name)
> > +{
> > +       uint16_t namelen;
> > +
> > +       if (len < 12)
> > +               return FALSE;
> > +
> > +       memset(name, 0, NAME_MAX_LEN);
> > +       namelen = MIN(get_be16(&operands[12]), NAME_MAX_LEN - 1);
> > +       if (namelen > 0)
> > +               memcpy(name, &operands[14], namelen);
> > +
> > +       return TRUE;
> > +}
> 
> Rather than creating yet another file how about hosting this under
> shared/util.h directly? It already depends on it anyway, we could got
> with something like strntoutf8 or a similar function that checks the
> length, etc, actually be maybe better to do it under
> util_iov_pull_utf8(iov, len, str, str_max_len) so we can load the pdu
> into the iov then use iov_pull_mem, etc, to verify that we have
> enough
> bytes in a generic manner.

That would be nice follow-up work to be done, but this patch is
specifically about being able to test the out-of-bounds access caused
by those 2 portions of code in patch #2.

Then the fix is applied in patch #3.

Finally, we could optimise/clean this up and remove that parsing code,
but that would be in a 4th patch.

Re: [BlueZ, v2 2/3] unit: Adapt poc_*_oob.c test into a new test

[Bastien Nocera]

On Wed, 2026-08-05 at 12:44 -0400, Luiz Augusto von Dentz wrote:
> Hi Bastien,
> 
> On Wed, Aug 5, 2026 at 3:59 AM Bastien Nocera <hadess@hadess.net>
> wrote:
> > 
> > On Tue, 2026-08-04 at 12:46 -0400, Luiz Augusto von Dentz wrote:
> > > Hi Bastien,
> > > 
> > > On Tue, Aug 4, 2026 at 10:31 AM Bastien Nocera
> > > <hadess@hadess.net>
> > > wrote:
> > > > 
> > > > Adapt poc_avrcp_oob.c and poc_folder_oob.c into unit tests.
> > > > 
> > > > Co-authored-by: Elman Shahbazov <shahbazovelman97@gmail.com>
> > > > ---
> > > >  Makefile.am           |  9 +++++
> > > >  unit/test-avrcp-sec.c | 76
> > > > +++++++++++++++++++++++++++++++++++++++++++
> > > >  2 files changed, 85 insertions(+)
> > > >  create mode 100644 unit/test-avrcp-sec.c
> > > > 
> > > > diff --git a/Makefile.am b/Makefile.am
> > > > index 19c468d3a504..e3baa4155c1f 100644
> > > > --- a/Makefile.am
> > > > +++ b/Makefile.am
> > > > @@ -660,6 +660,15 @@ unit_test_avrcp_SOURCES = unit/test-
> > > > avrcp.c \
> > > >  unit_test_avrcp_LDADD = lib/libbluetooth-internal.la \
> > > >                                 src/libshared-glib.la
> > > > $(GLIB_LIBS)
> > > > 
> > > > +unit_tests += unit/test-avrcp-sec
> > > > +
> > > > +unit_test_avrcp_sec_SOURCES = unit/test-avrcp-sec.c \
> > > > +                       profiles/audio/avrcp-parse.c \
> > > > +                       profiles/audio/avrcp-parse.h \
> > > > +                       src/log.h src/log.c
> > > > +unit_test_avrcp_sec_LDADD = lib/libbluetooth-internal.la \
> > > > +                               src/libshared-glib.la
> > > > $(GLIB_LIBS)
> > > 
> > > Any reason why this couldn't live inside test-avrcp.c?
> > 
> > Because most of the code in test-avrcp.c is mocked in unit/avrcp-
> > lib.[ch] and conflicts with code from profiles/audio/avrcp.c. I
> > really
> > did try...
> 
> Ok, well then perhaps we should start testing what we really use in
> the avrcp plugin rather than the removed Android code, or we could
> have a bigger task: converting this type of code to
> src/shared/avrcp.c
> (e.g. bt_avrcp) so it can be unit tested.

I figured that this code would actually test *something*, but it looks
like I was wrong.

I'll re-send the patch to remove the dead code for unit/avrcp.c, and
send a follow-up patch to remove this code here, and reinstate it with
this test.

Right now, "make coverage" says that profiles/audio has no coverage at
all...

> > I can change the name of the test if needed.
> 
> I'm not concerned about the name, I'm concerned about the number of
> unit test files which could otherwise be in a single file instead of
> duplicating a lot of context.
> 
> > > 
> > > >  unit_tests += unit/test-hfp
> > > > 
> > > >  unit_test_hfp_SOURCES = unit/test-hfp.c
> > > > diff --git a/unit/test-avrcp-sec.c b/unit/test-avrcp-sec.c
> > > > new file mode 100644
> > > > index 000000000000..a100b2d68e35
> > > > --- /dev/null
> > > > +++ b/unit/test-avrcp-sec.c
> > > > @@ -0,0 +1,76 @@
> > > > +// SPDX-License-Identifier: GPL-2.0-or-later
> > > > +/*
> > > > + *
> > > > + *  BlueZ - Bluetooth protocol stack for Linux
> > > > + *
> > > > + *  Copyright (C) 2026  Red Hat Inc.
> > > > + *
> > > > + *
> > > > + */
> > > > +
> > > > +#ifdef HAVE_CONFIG_H
> > > > +#include <config.h>
> > > > +#endif
> > > > +
> > > > +#include <glib.h>
> > > > +
> > > > +#include "src/shared/util.h"
> > > > +#include "src/shared/tester.h"
> > > > +#include "src/log.h"
> > > > +
> > > > +#include "profiles/audio/avrcp-parse.h"
> > > > +
> > > > +static void avrcp_element_name_oob(gconstpointer data)
> > > > +{
> > > > +       char name[255];
> > > > +       uint16_t namesize;
> > > > +       gboolean ret;
> > > > +
> > > > +       /* Crafting a malicious payload.
> > > > +        * Actual packet length (len) = 14 bytes */
> > > > +       uint8_t malicious_packet[14] = {0};
> > > > +
> > > > +       /* Specify namesize = 1000 (0x03E8 in Big Endian) at
> > > > offset
> > > > 11 */
> > > > +       malicious_packet[11] = 0x03;
> > > > +       malicious_packet[12] = 0xE8;
> > > > +
> > > > +       /* Launching the PoC. We transmit a 14-byte packet, but
> > > > namesize=1000... */
> > > > +       ret = parse_media_element_name(malicious_packet,
> > > > sizeof(malicious_packet),
> > > > +                                      name, &namesize);
> > > > +       if (ret)
> > > > +               tester_test_passed();
> > > > +       else
> > > > +               tester_test_failed();
> > > > +}
> > > > +
> > > > +static void avrcp_folder_name_oob(gconstpointer data)
> > > > +{
> > > > +       char name[255];
> > > > +       gboolean ret;
> > > > +
> > > > +       /* Crafting a malicious payload.
> > > > +        * Actual packet length (len) = 14 bytes */
> > > > +       uint8_t malicious_packet[14] = {0};
> > > > +
> > > > +       /* Specify namesize = 1000 (0x03E8 in Big Endian) at
> > > > offset
> > > > 12 */
> > > > +       malicious_packet[12] = 0x03;
> > > > +       malicious_packet[13] = 0xE8;
> > > > +
> > > > +       /* Launching the PoC. We transmit a 14-byte packet, but
> > > > namesize=1000... */
> > > > +       ret = parse_media_folder_name(malicious_packet,
> > > > sizeof(malicious_packet),
> > > > +                                     name);
> > > > +       if (ret)
> > > > +               tester_test_passed();
> > > > +       else
> > > > +               tester_test_failed();
> > > > +}
> > > > +
> > > > +int main(int argc, char *argv[])
> > > > +{
> > > > +       tester_init(&argc, &argv);
> > > > +
> > > > +       tester_add("/avrcp-element-name-oob", NULL, NULL,
> > > > avrcp_element_name_oob, NULL);
> > > > +       tester_add("/avrcp-folder-name-oob", NULL, NULL,
> > > > avrcp_folder_name_oob, NULL);
> > > > +
> > > > +       return tester_run();
> > > > +}
> > > > --
> > > > 2.55.0
> > > > 
> > > > 
> > > 
> 
> 

Re: [BlueZ, v2 1/3] avrcp: Split off name parsing from parse_*_element()

[Luiz Augusto von Dentz]

Hi Bastien,

On Thu, Aug 6, 2026 at 5:59 AM Bastien Nocera <hadess@hadess.net> wrote:
>
> On Wed, 2026-08-05 at 13:12 -0400, Luiz Augusto von Dentz wrote:
> > Hi Bastien,
> >
> > On Tue, Aug 4, 2026 at 10:31 AM Bastien Nocera <hadess@hadess.net>
> > wrote:
> > >
> > > This will allow us to use the name extraction code in
> > > parse_media_element() and parse_folder_element() separately, such
> > > as in tests.
> > > ---
> > >  Makefile.plugins             |  1 +
> > >  profiles/audio/avrcp-parse.c | 47
> > > ++++++++++++++++++++++++++++++++++++
> > >  profiles/audio/avrcp-parse.h | 19 +++++++++++++++
> > >  profiles/audio/avrcp.c       | 26 +++++---------------
> > >  4 files changed, 73 insertions(+), 20 deletions(-)
> > >  create mode 100644 profiles/audio/avrcp-parse.c
> > >  create mode 100644 profiles/audio/avrcp-parse.h
> > >
> > > diff --git a/Makefile.plugins b/Makefile.plugins
> > > index ac667beda847..a505fcd6691f 100644
> > > --- a/Makefile.plugins
> > > +++ b/Makefile.plugins
> > > @@ -37,6 +37,7 @@ builtin_modules += avrcp
> > >  builtin_sources += profiles/audio/control.h
> > > profiles/audio/control.c \
> > >                         profiles/audio/avctp.h
> > > profiles/audio/avctp.c \
> > >                         profiles/audio/avrcp.h
> > > profiles/audio/avrcp.c \
> > > +                       profiles/audio/avrcp-parse.h
> > > profiles/audio/avrcp-parse.c \
> > >                         profiles/audio/avrcp-player.c
> > >  endif
> > >
> > > diff --git a/profiles/audio/avrcp-parse.c b/profiles/audio/avrcp-
> > > parse.c
> > > new file mode 100644
> > > index 000000000000..d3d0a070a4da
> > > --- /dev/null
> > > +++ b/profiles/audio/avrcp-parse.c
> > > @@ -0,0 +1,47 @@
> > > +// SPDX-License-Identifier: GPL-2.0-or-later
> > > +/*
> > > + *
> > > + *  BlueZ - Bluetooth protocol stack for Linux
> > > + *
> > > + *  Copyright (C) 2026  Red Hat Inc.
> > > + *
> > > + *
> > > + */
> > > +
> > > +#include "avrcp-parse.h"
> > > +#include "src/shared/util.h"
> > > +
> > > +gboolean parse_media_element_name(uint8_t *operands, uint16_t len,
> > > +                                        char *name, uint16_t
> > > *namesize)
> > > +{
> > > +       uint16_t namelen;
> > > +
> > > +       if (len < 13)
> > > +               return FALSE;
> > > +
> > > +       memset(name, 0, NAME_MAX_LEN);
> > > +       *namesize = get_be16(&operands[11]);
> > > +       namelen = MIN(*namesize, NAME_MAX_LEN - 1);
> > > +       if (namelen > 0) {
> > > +               memcpy(name, &operands[13], namelen);
> > > +               strtoutf8(name, namelen);
> > > +       }
> > > +
> > > +       return TRUE;
> > > +}
> > > +
> > > +gboolean parse_media_folder_name(uint8_t *operands, uint16_t len,
> > > +                                       char *name)
> > > +{
> > > +       uint16_t namelen;
> > > +
> > > +       if (len < 12)
> > > +               return FALSE;
> > > +
> > > +       memset(name, 0, NAME_MAX_LEN);
> > > +       namelen = MIN(get_be16(&operands[12]), NAME_MAX_LEN - 1);
> > > +       if (namelen > 0)
> > > +               memcpy(name, &operands[14], namelen);
> > > +
> > > +       return TRUE;
> > > +}
> >
> > Rather than creating yet another file how about hosting this under
> > shared/util.h directly? It already depends on it anyway, we could got
> > with something like strntoutf8 or a similar function that checks the
> > length, etc, actually be maybe better to do it under
> > util_iov_pull_utf8(iov, len, str, str_max_len) so we can load the pdu
> > into the iov then use iov_pull_mem, etc, to verify that we have
> > enough
> > bytes in a generic manner.
>
> That would be nice follow-up work to be done, but this patch is
> specifically about being able to test the out-of-bounds access caused
> by those 2 portions of code in patch #2.
>
> Then the fix is applied in patch #3.
>
> Finally, we could optimise/clean this up and remove that parsing code,
> but that would be in a 4th patch.

I don't think this is the best approach because test-avrcp is testing
the wrong implementation, so that needs rework. I'd go straight to
shared/util helper function which would allow us to create test cases
immediately without changing a bunch of things and removing
test-avrcp.c in the process.

-- 
Luiz Augusto von Dentz