[BlueZ, v3 0/9] 3 SDP XML security fixes

[BlueZ, v3 0/9] 3 SDP XML security fixes

[Bastien Nocera]

Changes since v2:
- Reorder commits so that unit tests are added after the fix, so as
  not to break "git bisect"
- Add "Fix stack overflow when converting large sequences to XML" patch
- Fix crash in unit test when test files are missing

Changes since v1:
- Added "Fix memory leak when adding duplicate attributes"

Bastien Nocera (9):
  unit: Add test for sdp_xml_parse_record()
  sdp-xml: Fix crash caused by type confusion when parsing crafted SDP
    XML
  unit: Add test for sdp-xml type-confusion bug
  sdp-xml: Fix memory leak when adding duplicate attributes
  unit: Add test for sdp-xml duplicate attribute bug
  sdp-xml: Optimise parsing large sequences
  unit: Add test for slow element_end() append
  sdp-xml: Fix stack overflow when converting large sequences to XML
  unit: Add convert_sdp_record_to_xml() to SDP XML testing

 Makefile.am                                   |  10 ++
 src/sdp-xml.c                                 |  55 ++++++-
 unit/sdp-xml/Bluetooth_HID-sdp_record.xml     | 123 ++++++++++++++++
 .../compute-seq-size-type-confusion.xml       |   9 ++
 unit/sdp-xml/duplicate-attribute.xml          |   4 +
 unit/sdp-xml/qt-SerialPortSDPRecord.xml       |  57 +++++++
 unit/test-sdp-xml.c                           | 139 ++++++++++++++++++
 7 files changed, 390 insertions(+), 7 deletions(-)
 create mode 100644 unit/sdp-xml/Bluetooth_HID-sdp_record.xml
 create mode 100644 unit/sdp-xml/compute-seq-size-type-confusion.xml
 create mode 100644 unit/sdp-xml/duplicate-attribute.xml
 create mode 100644 unit/sdp-xml/qt-SerialPortSDPRecord.xml
 create mode 100644 unit/test-sdp-xml.c

-- 
2.55.0

[BlueZ, v3 1/9] unit: Add test for sdp_xml_parse_record()

[Bastien Nocera]

This adds 2 example XML files from other repositories, under a fair use
license exception.
---
 Makefile.am                               |  10 ++
 unit/sdp-xml/Bluetooth_HID-sdp_record.xml | 123 ++++++++++++++++++++++
 unit/sdp-xml/qt-SerialPortSDPRecord.xml   |  57 ++++++++++
 unit/test-sdp-xml.c                       |  82 +++++++++++++++
 4 files changed, 272 insertions(+)
 create mode 100644 unit/sdp-xml/Bluetooth_HID-sdp_record.xml
 create mode 100644 unit/sdp-xml/qt-SerialPortSDPRecord.xml
 create mode 100644 unit/test-sdp-xml.c

diff --git a/Makefile.am b/Makefile.am
index 19c468d3a504..1c62ee66bdce 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -637,6 +637,16 @@ unit_test_sdp_SOURCES = unit/test-sdp.c \
 unit_test_sdp_LDADD = lib/libbluetooth-internal.la \
 				src/libshared-glib.la $(GLIB_LIBS)
 
+unit_tests += unit/test-sdp-xml
+
+unit_test_sdp_xml_SOURCES = unit/test-sdp-xml.c \
+				src/sdp-xml.c src/sdp-xml.h \
+				src/log.h src/log.c
+unit_test_sdp_xml_LDADD = lib/libbluetooth-internal.la \
+				src/libshared-glib.la $(GLIB_LIBS)
+unit_test_sdp_xml_CFLAGS = $(AM_CFLAGS) $(GLIB_CFLAGS) -DTOP_SRCDIR=\""$(srcdir)"\"
+unit_test_sdp_xml_CPPFLAGS = -I$(srcdir)/lib
+
 unit_tests += unit/test-avdtp
 
 unit_test_avdtp_SOURCES = unit/test-avdtp.c \
diff --git a/unit/sdp-xml/Bluetooth_HID-sdp_record.xml b/unit/sdp-xml/Bluetooth_HID-sdp_record.xml
new file mode 100644
index 000000000000..687b0b15b520
--- /dev/null
+++ b/unit/sdp-xml/Bluetooth_HID-sdp_record.xml
@@ -0,0 +1,123 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+
+<!--
+
+ From: https://github.com/AnesBenmerzoug/Bluetooth_HID/blob/master/sdp_record.xml
+ A description of these fields can be found in the following links:
+ http://www.bluecove.org/bluecove/apidocs/javax/bluetooth/ServiceRecord.html
+ https://www.bluetooth.com/specifications/assigned-numbers/service-discovery
+
+ -->
+
+<record>
+	<attribute id="0x0001"> <!-- Service Class ID List -->
+		<sequence>
+			<uuid value="0x1124" /> <!-- Human Interface Device -->
+		</sequence>
+	</attribute>
+	<attribute id="0x0004"> <!-- Protocol Descriptor List -->
+		<sequence>
+			<sequence>
+				<uuid value="0x0100" /> <!-- L2CAP -->
+				<uint16 value="0x0011" /> <!-- HIDP -->
+			</sequence>
+			<sequence>
+				<uuid value="0x0011" /> <!-- HIDP -->
+			</sequence>
+		</sequence>
+	</attribute>
+	<attribute id="0x0005"> <!-- Browse Group List -->
+		<sequence>
+			<uuid value="0x1002" />
+		</sequence>
+	</attribute>
+	<attribute id="0x0006"> <!-- Language Based Attribute ID List -->
+		<sequence>
+			<uint16 value="0x656e" /> <!-- code_ISO639 -->
+			<uint16 value="0x006a" /> <!-- encoding -->
+			<uint16 value="0x0100" /> <!-- base_offset -->
+		</sequence>
+	</attribute>
+	<attribute id="0x0009"> <!-- Bluetooth Profile Descriptor List -->
+		<sequence>
+			<sequence>
+				<uuid value="0x1124" /> <!-- Human Interface Device -->
+				<uint16 value="0x0100" /> <!-- L2CAP -->
+			</sequence>
+		</sequence>
+	</attribute>
+	<attribute id="0x000d"> <!-- Additional Protocol Descriptor Lists -->
+		<sequence>
+			<sequence>
+				<sequence>
+					<uuid value="0x0100" /> <!-- L2CAP -->
+					<uint16 value="0x0013" />
+				</sequence>
+				<sequence>
+					<uuid value="0x0011" /> <!-- HIDP -->
+				</sequence>
+			</sequence>
+		</sequence>
+	</attribute>
+	<attribute id="0x0100">
+		<text value="Bluetooth_Keyboard/Mouse" />
+	</attribute>
+	<attribute id="0x0101">
+		<text value="USB > BT Keyboard/Mouse" />
+	</attribute>
+	<attribute id="0x0102">
+		<text value="Raspberry Pi 3" />
+	</attribute>
+	<attribute id="0x0200">
+		<uint16 value="0x0100" />
+	</attribute>
+	<attribute id="0x0201">
+		<uint16 value="0x0111" />
+	</attribute>
+	<attribute id="0x0202">
+		<uint8 value="0x40" />
+	</attribute>
+	<attribute id="0x0203">
+		<uint8 value="0x00" />
+	</attribute>
+	<attribute id="0x0204">
+		<boolean value="false" />
+	</attribute>
+	<attribute id="0x0205">
+		<boolean value="false" />
+	</attribute>
+	<attribute id="0x0206">
+		<sequence>
+			<sequence>
+				<uint8 value="0x22" />
+				<text encoding="hex" value="05010906A1018501A100050719E029E71500250175019508810295017508810195087508150025650507190029658100C0C005010902A10185020901A1000509190129031500250175019503810275059501810105010930093109381581257F750895038106C0C0"/>
+			</sequence>
+		</sequence>
+	</attribute>
+	<attribute id="0x0207">
+		<sequence>
+			<sequence>
+				<uint16 value="0x0409" />
+				<uint16 value="0x0100" />
+			</sequence>
+		</sequence>
+	</attribute>
+	<attribute id="0x020b">
+		<uint16 value="0x0100" />
+	</attribute>
+	<attribute id="0x020c">
+		<uint16 value="0x0c80" />
+	</attribute>
+	<attribute id="0x020d">
+		<boolean value="true" />
+	</attribute>
+	<attribute id="0x020e">
+		<boolean value="false" />
+	</attribute>
+	<attribute id="0x020f">
+		<uint16 value="0x0640" />
+	</attribute>
+	<attribute id="0x0210">
+		<uint16 value="0x0320" />
+	</attribute>
+</record>
diff --git a/unit/sdp-xml/qt-SerialPortSDPRecord.xml b/unit/sdp-xml/qt-SerialPortSDPRecord.xml
new file mode 100644
index 000000000000..1f62ccf11a41
--- /dev/null
+++ b/unit/sdp-xml/qt-SerialPortSDPRecord.xml
@@ -0,0 +1,57 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+
+<!--
+ This is an XML file describing an SDP service record for a Serial Port
+ service.
+
+ You can use the linux sdptool command to create a file like this for
+ publishing your Bluetooth services. See the "Creating a Bluetooth service"
+ tutorial for more details.
+
+ From: https://radekp.github.io/qtmoko/api/bluetooth-bluetoothservice-serialportsdprecord-xml.html
+-->
+
+ <record>
+         <attribute id="0x0001">
+                 <sequence>
+                         <uuid value="0x1101" />
+                 </sequence>
+         </attribute>
+         <attribute id="0x0004">
+                 <sequence>
+                         <sequence>
+                                 <uuid value="0x0100" />
+                         </sequence>
+                         <sequence>
+                                 <uuid value="0x0003" />
+                                 <uint8 value="0x05" />
+                         </sequence>
+                 </sequence>
+         </attribute>
+         <attribute id="0x0005">
+                 <sequence>
+                         <uuid value="0x1002" />
+                 </sequence>
+         </attribute>
+         <attribute id="0x0006">
+                 <sequence>
+                         <uint16 value="0x656e" />
+                         <uint16 value="0x006a" />
+                         <uint16 value="0x0100" />
+                 </sequence>
+         </attribute>
+         <attribute id="0x0009">
+                 <sequence>
+                         <sequence>
+                                 <uuid value="0x1101" />
+                                 <uint16 value="0x0100" />
+                         </sequence>
+                 </sequence>
+         </attribute>
+         <attribute id="0x0100">
+                 <text value="Serial Port" />
+         </attribute>
+         <attribute id="0x0101">
+                 <text value="COM Port" />
+         </attribute>
+ </record>
diff --git a/unit/test-sdp-xml.c b/unit/test-sdp-xml.c
new file mode 100644
index 000000000000..11a83951678d
--- /dev/null
+++ b/unit/test-sdp-xml.c
@@ -0,0 +1,82 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2026  Intel Corporation. All rights reserved.
+ *
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <glib.h>
+
+#include "bluetooth/sdp.h"
+#include "bluetooth/sdp_lib.h"
+
+#include "src/shared/util.h"
+#include "src/shared/tester.h"
+#include "src/log.h"
+#include "src/sdp-xml.h"
+
+struct test_data {
+	GString *s;
+	char *filename;
+	gboolean expected_result;
+};
+
+static void parse_xml(gconstpointer data, gsize len, gboolean expected_result)
+{
+	sdp_record_t *rec = NULL;
+	gboolean ret;
+
+	rec = sdp_xml_parse_record(data, len);
+	ret = rec ? TRUE : FALSE;
+	if (ret == expected_result)
+		tester_test_passed();
+	else
+		tester_test_failed();
+	if (rec)
+		sdp_record_free(rec);
+}
+
+static void parse_xml_for_filename(gconstpointer data)
+{
+	struct test_data *t = (struct test_data *) data;
+	char *path = NULL;
+	GError *error = NULL;
+	char *contents = NULL;
+	gsize len;
+
+	path = g_build_filename(TOP_SRCDIR, "unit", "sdp-xml", t->filename, NULL);
+	if (!g_file_get_contents(path, &contents, &len, &error)) {
+		g_warning("Can't load file '%s': %s", path, error->message);
+		g_free(path);
+		g_error_free(error);
+		tester_test_failed();
+		return;
+	}
+	parse_xml(contents, len, t->expected_result);
+	g_free(contents);
+	g_free(path);
+}
+
+#define DEFINE_TEST(fname, res)							\
+	data.expected_result = res;						\
+	data.filename = fname;							\
+	tester_add("/" fname, &data, NULL, parse_xml_for_filename, NULL);
+
+int main(int argc, char *argv[])
+{
+	struct test_data data;
+
+	tester_init(&argc, &argv);
+
+	DEFINE_TEST("Bluetooth_HID-sdp_record.xml", TRUE);
+	DEFINE_TEST("qt-SerialPortSDPRecord.xml", TRUE);
+
+	return tester_run();
+}
-- 
2.55.0

[BlueZ, v3 2/9] sdp-xml: Fix crash caused by type confusion when parsing crafted SDP XML

[Bastien Nocera]

When element_end() processes </attribute>, it frees ctx_data->stack_head
and clears the stack even if parsing is still nested inside a parent
container.

If a crafted ServiceRecord places a nested <attribute> inside <sequence>,
a later sibling scalar element such as <uint64> can become the new stack
head. When the closing </sequence> is then processed, compute_seq_size()
is reached without first validating that the current node is actually
a sequence.

sdp_data_t.val stores both scalar members such as uint64 and the
dataseq pointer in the same union. As a result, attacker-controlled
scalar data can be reinterpreted as a linked-list pointer and traversed
until bluetoothd crashes.

See https://github.com/bluez/bluez/security/advisories/GHSA-7mmr-gwqx-vc34

Reported-by: Aisle Research
Co-authored-by: Aisle Research
---
 src/sdp-xml.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/src/sdp-xml.c b/src/sdp-xml.c
index e5b30e88505f..c8f9ed013b29 100644
--- a/src/sdp-xml.c
+++ b/src/sdp-xml.c
@@ -529,7 +529,9 @@ static void element_end(GMarkupParseContext *context,
 		return;
 
 	if (!strcmp(element_name, "attribute")) {
-		if (ctx_data->stack_head && ctx_data->stack_head->data) {
+		/* Attributes are expected at top-level record scope. */
+		if (ctx_data->stack_head && ctx_data->stack_head->data &&
+		    ctx_data->stack_head->next == NULL) {
 			int ret = sdp_attr_add(ctx_data->record, ctx_data->attr_id,
 							ctx_data->stack_head->data);
 			if (ret == -1)
@@ -539,6 +541,11 @@ static void element_end(GMarkupParseContext *context,
 			ctx_data->stack_head->data = NULL;
 			sdp_xml_data_free(ctx_data->stack_head);
 			ctx_data->stack_head = NULL;
+		} else if (ctx_data->stack_head && ctx_data->stack_head->next) {
+			g_set_error(err, G_MARKUP_ERROR,
+				    G_MARKUP_ERROR_INVALID_CONTENT,
+				    "Nested <attribute> is invalid");
+			return;
 		} else {
 			DBG("No data for attribute 0x%04x", ctx_data->attr_id);
 		}
@@ -558,6 +565,13 @@ static void element_end(GMarkupParseContext *context,
 	}
 
 	if (!strcmp(element_name, "sequence")) {
+		if (!SDP_IS_SEQ(ctx_data->stack_head->data->dtd)) {
+			g_set_error(err, G_MARKUP_ERROR,
+				    G_MARKUP_ERROR_INVALID_CONTENT,
+				    "Mismatched </sequence> close");
+			return;
+		}
+
 		ctx_data->stack_head->data->unitSize = compute_seq_size(ctx_data->stack_head->data);
 
 		if (ctx_data->stack_head->data->unitSize > USHRT_MAX) {
@@ -570,6 +584,13 @@ static void element_end(GMarkupParseContext *context,
 			ctx_data->stack_head->data->unitSize += sizeof(uint8_t);
 		}
 	} else if (!strcmp(element_name, "alternate")) {
+		if (!SDP_IS_ALT(ctx_data->stack_head->data->dtd)) {
+			g_set_error(err, G_MARKUP_ERROR,
+				    G_MARKUP_ERROR_INVALID_CONTENT,
+				    "Mismatched </alternate> close");
+			return;
+		}
+
 		ctx_data->stack_head->data->unitSize = compute_seq_size(ctx_data->stack_head->data);
 
 		if (ctx_data->stack_head->data->unitSize > USHRT_MAX) {
-- 
2.55.0

[BlueZ, v3 3/9] unit: Add test for sdp-xml type-confusion bug

[Bastien Nocera]

See https://github.com/bluez/bluez/security/advisories/GHSA-7mmr-gwqx-vc34

Co-authored-by: Aisle Research
---
 unit/sdp-xml/compute-seq-size-type-confusion.xml | 9 +++++++++
 unit/test-sdp-xml.c                              | 2 ++
 2 files changed, 11 insertions(+)
 create mode 100644 unit/sdp-xml/compute-seq-size-type-confusion.xml

diff --git a/unit/sdp-xml/compute-seq-size-type-confusion.xml b/unit/sdp-xml/compute-seq-size-type-confusion.xml
new file mode 100644
index 000000000000..41d6478af276
--- /dev/null
+++ b/unit/sdp-xml/compute-seq-size-type-confusion.xml
@@ -0,0 +1,9 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<record>
+<attribute id="0x0001">
+<sequence>
+<attribute id="0x0002"><uint8 value="0x01"/></attribute>
+<uint64 value="0x4141414141414141"/>
+</sequence>
+</attribute>
+</record>
diff --git a/unit/test-sdp-xml.c b/unit/test-sdp-xml.c
index 11a83951678d..1e3123892f95 100644
--- a/unit/test-sdp-xml.c
+++ b/unit/test-sdp-xml.c
@@ -77,6 +77,8 @@ int main(int argc, char *argv[])
 
 	DEFINE_TEST("Bluetooth_HID-sdp_record.xml", TRUE);
 	DEFINE_TEST("qt-SerialPortSDPRecord.xml", TRUE);
+	/* From https://github.com/bluez/bluez/security/advisories/GHSA-7mmr-gwqx-vc34 */
+	DEFINE_TEST("compute-seq-size-type-confusion.xml", FALSE);
 
 	return tester_run();
 }
-- 
2.55.0

[BlueZ, v3 4/9] sdp-xml: Fix memory leak when adding duplicate attributes

[Bastien Nocera]

When sdp_attr_add() fails because an attribute is duplicated, don't
unset its pointer in the parsing context data. As the attribute wasn't
added to the record, the ownership of the attribute didn't get passed
to the record either.

Don't set the pointer to NULL so it gets freed when cleaning up the
context.

Fixes those 2 ASan warnings:
Direct leak of 48 byte(s) in 1 object(s) allocated from:
    #0 0x7f896a8ef24f in calloc (/lib64/libasan.so.8+0xef24f) (BuildId: 5395ec74f54d9ec7bf97c06583dd39a96c230822)
    #1 0x562042b747f2 in sdp_data_alloc_with_length lib/bluetooth/sdp.c:350

Indirect leak of 2 byte(s) in 1 object(s) allocated from:
    #0 0x7f896a8ef24f in calloc (/lib64/libasan.so.8+0xef24f) (BuildId: 5395ec74f54d9ec7bf97c06583dd39a96c230822)
    #1 0x562042b74bd7 in sdp_data_alloc_with_length lib/bluetooth/sdp.c:425

See https://github.com/bluez/bluez/security/advisories/GHSA-75v6-6q44-57hc

Reported-by: Aisle Research
Co-authored-by: Aisle Research
---
 src/sdp-xml.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/sdp-xml.c b/src/sdp-xml.c
index c8f9ed013b29..816d19611f8b 100644
--- a/src/sdp-xml.c
+++ b/src/sdp-xml.c
@@ -537,8 +537,11 @@ static void element_end(GMarkupParseContext *context,
 			if (ret == -1)
 				DBG("Could not add attribute 0x%04x",
 							ctx_data->attr_id);
+			else {
+				/* ownership transferred to record */
+				ctx_data->stack_head->data = NULL;
+			}
 
-			ctx_data->stack_head->data = NULL;
 			sdp_xml_data_free(ctx_data->stack_head);
 			ctx_data->stack_head = NULL;
 		} else if (ctx_data->stack_head && ctx_data->stack_head->next) {
-- 
2.55.0

[BlueZ, v3 5/9] unit: Add test for sdp-xml duplicate attribute bug

[Bastien Nocera]

See https://github.com/bluez/bluez/security/advisories/GHSA-75v6-6q44-57hc

Co-authored-by: Aisle Research
---
 unit/sdp-xml/duplicate-attribute.xml | 4 ++++
 unit/test-sdp-xml.c                  | 2 ++
 2 files changed, 6 insertions(+)
 create mode 100644 unit/sdp-xml/duplicate-attribute.xml

diff --git a/unit/sdp-xml/duplicate-attribute.xml b/unit/sdp-xml/duplicate-attribute.xml
new file mode 100644
index 000000000000..a30bc59d94ff
--- /dev/null
+++ b/unit/sdp-xml/duplicate-attribute.xml
@@ -0,0 +1,4 @@
+<record>
+<attribute id="0x0001"><text value="A"/></attribute>
+<attribute id="0x0001"><text value="B"/></attribute>
+</record>
diff --git a/unit/test-sdp-xml.c b/unit/test-sdp-xml.c
index 1e3123892f95..2794170dc153 100644
--- a/unit/test-sdp-xml.c
+++ b/unit/test-sdp-xml.c
@@ -79,6 +79,8 @@ int main(int argc, char *argv[])
 	DEFINE_TEST("qt-SerialPortSDPRecord.xml", TRUE);
 	/* From https://github.com/bluez/bluez/security/advisories/GHSA-7mmr-gwqx-vc34 */
 	DEFINE_TEST("compute-seq-size-type-confusion.xml", FALSE);
+	/* From https://github.com/bluez/bluez/security/advisories/GHSA-75v6-6q44-57hc */
+	DEFINE_TEST("duplicate-attribute.xml", TRUE);
 
 	return tester_run();
 }
-- 
2.55.0

[BlueZ, v3 6/9] sdp-xml: Optimise parsing large sequences

[Bastien Nocera]

SDP sequences are stored as single-linked lists, so appending members
to a sequence requires finding the tail of the list before the
insertion.

Finding the tail of the list always starts at the beginning of the list,
so takes longer and longer as the list grows bigger.

Keep track of the tail to avoid that problem. This cuts down the
sequence_on_squared() test from around 3 to 4 seconds to less than
0.1 seconds.
---
 src/sdp-xml.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/src/sdp-xml.c b/src/sdp-xml.c
index 816d19611f8b..97fb8b0c1af0 100644
--- a/src/sdp-xml.c
+++ b/src/sdp-xml.c
@@ -44,6 +44,7 @@ struct sdp_xml_data {
 	char type;			/* 0 = Text or Hexadecimal */
 	char *name;			/* Name, optional in the dtd */
 	/* TODO: What is it used for? */
+	sdp_data_t *tail;		/* Tail for O(1) dataseq append */
 };
 
 struct context_data {
@@ -609,6 +610,7 @@ static void element_end(GMarkupParseContext *context,
 
 	if (ctx_data->stack_head->next && ctx_data->stack_head->data &&
 					ctx_data->stack_head->next->data) {
+		sdp_data_t *tail;
 		switch (ctx_data->stack_head->next->data->dtd) {
 		case SDP_SEQ8:
 		case SDP_SEQ16:
@@ -616,10 +618,17 @@ static void element_end(GMarkupParseContext *context,
 		case SDP_ALT8:
 		case SDP_ALT16:
 		case SDP_ALT32:
-			ctx_data->stack_head->next->data->val.dataseq =
-				sdp_seq_append(ctx_data->stack_head->next->data->val.dataseq,
-								ctx_data->stack_head->data);
+			tail = ctx_data->stack_head->next->data->val.dataseq ?
+				ctx_data->stack_head->next->tail : NULL;
+			if (tail) {
+				sdp_seq_append(tail, ctx_data->stack_head->data);
+			} else {
+				ctx_data->stack_head->next->data->val.dataseq =
+					sdp_seq_append(NULL, ctx_data->stack_head->data);
+			}
+			ctx_data->stack_head->next->tail = ctx_data->stack_head->data;
 			ctx_data->stack_head->data = NULL;
+			ctx_data->stack_head->tail = NULL;
 			break;
 		}
 
-- 
2.55.0

[BlueZ, v3 7/9] unit: Add test for slow element_end() append

[Bastien Nocera]

This uses 40k iterations as this takes a visible amount of time on a
pretty fast desktop machine (3-4 secs on an i9 9900k).

See: https://github.com/bluez/bluez/security/advisories/GHSA-4p57-mrcv-r2jc
---
 unit/test-sdp-xml.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/unit/test-sdp-xml.c b/unit/test-sdp-xml.c
index 2794170dc153..0a4f9b0d3992 100644
--- a/unit/test-sdp-xml.c
+++ b/unit/test-sdp-xml.c
@@ -64,6 +64,46 @@ static void parse_xml_for_filename(gconstpointer data)
 	g_free(path);
 }
 
+#define XML_START								\
+	"<?xml version=\"1.0\" encoding=\"UTF-8\" ?>\n"				\
+	"<record>\n"								\
+	"	<attribute id=\"0x0004\">\n"					\
+	"		<sequence>"
+
+#define XML_VALUE								\
+	"			<uint8 value=\"0x01\" />\n"
+
+#define XML_END									\
+	"		</sequence>\n"						\
+	"	</attribute>\n"							\
+	"</record>"
+
+static void sequence_on_squared(gconstpointer data)
+{
+	struct test_data *t = (struct test_data *) data;
+	parse_xml(t->s->str, t->s->len, TRUE);
+}
+
+static void sequence_on_squared_setup(gconstpointer data)
+{
+	struct test_data *t = (struct test_data *) data;
+	guint i;
+
+	t->s = g_string_new(XML_START);
+	for (i = 0; i < 40000; i++)
+		t->s = g_string_append(t->s, XML_VALUE);
+	t->s = g_string_append(t->s, XML_END);
+
+	tester_setup_complete();
+}
+
+static void sequence_on_squared_teardown(gconstpointer data)
+{
+	struct test_data *t = (struct test_data *) data;
+	g_string_free(t->s, TRUE);
+	tester_teardown_complete();
+}
+
 #define DEFINE_TEST(fname, res)							\
 	data.expected_result = res;						\
 	data.filename = fname;							\
@@ -82,5 +122,10 @@ int main(int argc, char *argv[])
 	/* From https://github.com/bluez/bluez/security/advisories/GHSA-75v6-6q44-57hc */
 	DEFINE_TEST("duplicate-attribute.xml", TRUE);
 
+	tester_add("/sequence_on_squared", &data,
+		   sequence_on_squared_setup,
+		   sequence_on_squared,
+		   sequence_on_squared_teardown);
+
 	return tester_run();
 }
-- 
2.55.0

[BlueZ, v3 8/9] sdp-xml: Fix stack overflow when converting large sequences to XML

[Bastien Nocera]

Don't make convert_raw_data_to_xml() call itself recursively X times
if there are X elements in a sequence.

AddressSanitizer:DEADLYSIGNAL
=================================================================
==1684518==ERROR: AddressSanitizer: stack-overflow on address 0x7fff7fb40d98 (pc 0x7fbe1ee95c2b bp 0x7fff7fb41610 sp 0x7fff7fb40d70 T0)
    #0 0x7fbe1ee95c2b in printf_common(void*, char const*, __va_list_tag*) (/lib64/libasan.so.8+0x95c2b) (BuildId: 5395ec74f54d9ec7bf97c06583dd39a96c230822)
    #1 0x7fbe1eeb72d6 in vsnprintf (/lib64/libasan.so.8+0xb72d6) (BuildId: 5395ec74f54d9ec7bf97c06583dd39a96c230822)
    #2 0x7fbe1eeb94f4 in snprintf (/lib64/libasan.so.8+0xb94f4) (BuildId: 5395ec74f54d9ec7bf97c06583dd39a96c230822)
    #3 0x000000401fc2 in convert_raw_data_to_xml ../../../../Projects/jhbuild/bluez/src/sdp-xml.c:709
    #4 0x000000401c4f in convert_raw_data_to_xml ../../../../Projects/jhbuild/bluez/src/sdp-xml.c:994
[...]
    #246 0x000000401c4f in convert_raw_data_to_xml ../../../../Projects/jhbuild/bluez/src/sdp-xml.c:994

SUMMARY: AddressSanitizer: stack-overflow ../../../../Projects/jhbuild/bluez/src/sdp-xml.c:709 in convert_raw_data_to_xml
---
 src/sdp-xml.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/src/sdp-xml.c b/src/sdp-xml.c
index 97fb8b0c1af0..0dcb8a7df601 100644
--- a/src/sdp-xml.c
+++ b/src/sdp-xml.c
@@ -679,8 +679,10 @@ sdp_record_t *sdp_xml_parse_record(const char *data, int size)
 	return record;
 }
 
-
 static void convert_raw_data_to_xml(sdp_data_t *value, int indent_level,
+		void *data, void (*appender)(void *, const char *));
+
+static inline void convert_raw_data_to_xml_element(sdp_data_t *value, int indent_level,
 		void *data, void (*appender)(void *, const char *))
 {
 	int i, hex;
@@ -999,8 +1001,14 @@ static void convert_raw_data_to_xml(sdp_data_t *value, int indent_level,
 
 		break;
 	}
+}
 
-	convert_raw_data_to_xml(value->next, indent_level, data, appender);
+static void convert_raw_data_to_xml(sdp_data_t *value, int indent_level,
+		void *data, void (*appender)(void *, const char *))
+{
+	for (; value != NULL; value = value->next) {
+		convert_raw_data_to_xml_element(value, indent_level, data, appender);
+	}
 }
 
 struct conversion_data {
-- 
2.55.0

[BlueZ, v3 9/9] unit: Add convert_sdp_record_to_xml() to SDP XML testing

[Bastien Nocera]

This tests SDP binary to XML conversion, including whether a fix for
a stack overflow when dealing with large sequences, like in
sequence_on_squared(), works correctly.
---
 unit/test-sdp-xml.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/unit/test-sdp-xml.c b/unit/test-sdp-xml.c
index 0a4f9b0d3992..9ebe6ee5b912 100644
--- a/unit/test-sdp-xml.c
+++ b/unit/test-sdp-xml.c
@@ -28,6 +28,12 @@ struct test_data {
 	gboolean expected_result;
 };
 
+static void doprintf(void *data, const char *str)
+{
+	/* Do nothing for our tests */
+	/* printf("%s", str); */
+}
+
 static void parse_xml(gconstpointer data, gsize len, gboolean expected_result)
 {
 	sdp_record_t *rec = NULL;
@@ -39,8 +45,10 @@ static void parse_xml(gconstpointer data, gsize len, gboolean expected_result)
 		tester_test_passed();
 	else
 		tester_test_failed();
-	if (rec)
+	if (rec) {
+		convert_sdp_record_to_xml(rec, 0, doprintf);
 		sdp_record_free(rec);
+	}
 }
 
 static void parse_xml_for_filename(gconstpointer data)
-- 
2.55.0

RE: 3 SDP XML security fixes

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 13763 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=1144151

---Test result---

Test Summary:
CheckPatch                    FAIL      7.01 seconds
GitLint                       FAIL      5.28 seconds
BuildEll                      PASS      18.29 seconds
BluezMake                     PASS      655.00 seconds
MakeCheck                     PASS      18.42 seconds
MakeDistcheck                 FAIL      130.45 seconds
CheckValgrind                 PASS      222.33 seconds
CheckSmatch                   PASS      274.91 seconds
bluezmakeextell               PASS      96.01 seconds
IncrementalBuild              PASS      664.33 seconds
ScanBuild                     PASS      886.25 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
[BlueZ,v3,1/9] unit: Add test for sdp_xml_parse_record()
WARNING:LONG_LINE: line length of 82 exceeds 80 columns
#340: FILE: unit/test-sdp-xml.c:54:
+	path = g_build_filename(TOP_SRCDIR, "unit", "sdp-xml", t->filename, NULL);

WARNING:LONG_LINE: line length of 81 exceeds 80 columns
#353: FILE: unit/test-sdp-xml.c:67:
+#define DEFINE_TEST(fname, res)							\

WARNING:TRAILING_SEMICOLON: macros should not use a trailing semicolon
#353: FILE: unit/test-sdp-xml.c:67:
+#define DEFINE_TEST(fname, res)							\
+	data.expected_result = res;						\
+	data.filename = fname;							\
+	tester_add("/" fname, &data, NULL, parse_xml_for_filename, NULL);

WARNING:LONG_LINE: line length of 81 exceeds 80 columns
#354: FILE: unit/test-sdp-xml.c:68:
+	data.expected_result = res;						\

WARNING:LONG_LINE: line length of 81 exceeds 80 columns
#355: FILE: unit/test-sdp-xml.c:69:
+	data.filename = fname;							\

/github/workspace/src/patch/14743502.patch total: 0 errors, 5 warnings, 278 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/14743502.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,v3,2/9] sdp-xml: Fix crash caused by type confusion when parsing crafted SDP XML
ERROR:BAD_SIGN_OFF: Unrecognized email address: 'Aisle Research'
#75: 
Reported-by: Aisle Research

WARNING:BAD_SIGN_OFF: Non-standard signature: Co-authored-by:
#76: 
Co-authored-by: Aisle Research

ERROR:BAD_SIGN_OFF: Unrecognized email address: 'Aisle Research'
#76: 
Co-authored-by: Aisle Research

/github/workspace/src/patch/14743501.patch total: 2 errors, 1 warnings, 47 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/14743501.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,v3,3/9] unit: Add test for sdp-xml type-confusion bug
WARNING:BAD_SIGN_OFF: Non-standard signature: Co-authored-by:
#58: 
Co-authored-by: Aisle Research

ERROR:BAD_SIGN_OFF: Unrecognized email address: 'Aisle Research'
#58: 
Co-authored-by: Aisle Research

/github/workspace/src/patch/14743500.patch total: 1 errors, 1 warnings, 17 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/14743500.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,v3,4/9] sdp-xml: Fix memory leak when adding duplicate attributes
WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#67: 
    #0 0x7f896a8ef24f in calloc (/lib64/libasan.so.8+0xef24f) (BuildId: 5395ec74f54d9ec7bf97c06583dd39a96c230822)

ERROR:BAD_SIGN_OFF: Unrecognized email address: 'Aisle Research'
#76: 
Reported-by: Aisle Research

WARNING:BAD_SIGN_OFF: Non-standard signature: Co-authored-by:
#77: 
Co-authored-by: Aisle Research

ERROR:BAD_SIGN_OFF: Unrecognized email address: 'Aisle Research'
#77: 
Co-authored-by: Aisle Research

/github/workspace/src/patch/14743503.patch total: 2 errors, 2 warnings, 12 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/14743503.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,v3,5/9] unit: Add test for sdp-xml duplicate attribute bug
WARNING:BAD_SIGN_OFF: Non-standard signature: Co-authored-by:
#58: 
Co-authored-by: Aisle Research

ERROR:BAD_SIGN_OFF: Unrecognized email address: 'Aisle Research'
#58: 
Co-authored-by: Aisle Research

/github/workspace/src/patch/14743508.patch total: 1 errors, 1 warnings, 12 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/14743508.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,v3,6/9] sdp-xml: Optimise parsing large sequences
WARNING:LONG_LINE: line length of 81 exceeds 80 columns
#100: FILE: src/sdp-xml.c:624:
+				sdp_seq_append(tail, ctx_data->stack_head->data);

WARNING:LONG_LINE: line length of 89 exceeds 80 columns
#103: FILE: src/sdp-xml.c:627:
+					sdp_seq_append(NULL, ctx_data->stack_head->data);

WARNING:LONG_LINE: line length of 86 exceeds 80 columns
#105: FILE: src/sdp-xml.c:629:
+			ctx_data->stack_head->next->tail = ctx_data->stack_head->data;

/github/workspace/src/patch/14743504.patch total: 0 errors, 3 warnings, 34 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/14743504.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,v3,7/9] unit: Add test for slow element_end() append
WARNING:LONG_LINE: line length of 81 exceeds 80 columns
#72: FILE: unit/test-sdp-xml.c:67:
+#define XML_START								\

WARNING:LONG_LINE_STRING: line length of 81 exceeds 80 columns
#73: FILE: unit/test-sdp-xml.c:68:
+	"<?xml version=\"1.0\" encoding=\"UTF-8\" ?>\n"				\

WARNING:LONG_LINE_STRING: line length of 81 exceeds 80 columns
#74: FILE: unit/test-sdp-xml.c:69:
+	"<record>\n"								\

WARNING:LONG_LINE_STRING: line length of 81 exceeds 80 columns
#75: FILE: unit/test-sdp-xml.c:70:
+	"	<attribute id=\"0x0004\">\n"					\

WARNING:LONG_LINE: line length of 81 exceeds 80 columns
#78: FILE: unit/test-sdp-xml.c:73:
+#define XML_VALUE								\

WARNING:LONG_LINE: line length of 81 exceeds 80 columns
#81: FILE: unit/test-sdp-xml.c:76:
+#define XML_END									\

WARNING:LONG_LINE_STRING: line length of 81 exceeds 80 columns
#82: FILE: unit/test-sdp-xml.c:77:
+	"		</sequence>\n"						\

WARNING:LONG_LINE_STRING: line length of 81 exceeds 80 columns
#83: FILE: unit/test-sdp-xml.c:78:
+	"	</attribute>\n"							\

WARNING:LINE_SPACING: Missing a blank line after declarations
#89: FILE: unit/test-sdp-xml.c:84:
+	struct test_data *t = (struct test_data *) data;
+	parse_xml(t->s->str, t->s->len, TRUE);

WARNING:LINE_SPACING: Missing a blank line after declarations
#108: FILE: unit/test-sdp-xml.c:103:
+	struct test_data *t = (struct test_data *) data;
+	g_string_free(t->s, TRUE);

/github/workspace/src/patch/14743507.patch total: 0 errors, 10 warnings, 56 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/14743507.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,v3,8/9] sdp-xml: Fix stack overflow when converting large sequences to XML
WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#62: 
==1684518==ERROR: AddressSanitizer: stack-overflow on address 0x7fff7fb40d98 (pc 0x7fbe1ee95c2b bp 0x7fff7fb41610 sp 0x7fff7fb40d70 T0)

WARNING:LONG_LINE: line length of 87 exceeds 80 columns
#88: FILE: src/sdp-xml.c:685:
+static inline void convert_raw_data_to_xml_element(sdp_data_t *value, int indent_level,

WARNING:BRACES: braces {} are not necessary for single statement blocks
#102: FILE: src/sdp-xml.c:1009:
+	for (; value != NULL; value = value->next) {
+		convert_raw_data_to_xml_element(value, indent_level, data, appender);
+	}

WARNING:LONG_LINE: line length of 85 exceeds 80 columns
#103: FILE: src/sdp-xml.c:1010:
+		convert_raw_data_to_xml_element(value, indent_level, data, appender);

/github/workspace/src/patch/14743505.patch total: 0 errors, 4 warnings, 26 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/14743505.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.


##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[BlueZ,v3,2/9] sdp-xml: Fix crash caused by type confusion when parsing crafted SDP XML

1: T1 Title exceeds max length (87>80): "[BlueZ,v3,2/9] sdp-xml: Fix crash caused by type confusion when parsing crafted SDP XML"
[BlueZ,v3,4/9] sdp-xml: Fix memory leak when adding duplicate attributes

13: B1 Line exceeds max length (113>80): "    #0 0x7f896a8ef24f in calloc (/lib64/libasan.so.8+0xef24f) (BuildId: 5395ec74f54d9ec7bf97c06583dd39a96c230822)"
17: B1 Line exceeds max length (113>80): "    #0 0x7f896a8ef24f in calloc (/lib64/libasan.so.8+0xef24f) (BuildId: 5395ec74f54d9ec7bf97c06583dd39a96c230822)"
[BlueZ,v3,8/9] sdp-xml: Fix stack overflow when converting large sequences to XML

1: T1 Title exceeds max length (81>80): "[BlueZ,v3,8/9] sdp-xml: Fix stack overflow when converting large sequences to XML"
8: B1 Line exceeds max length (135>80): "==1684518==ERROR: AddressSanitizer: stack-overflow on address 0x7fff7fb40d98 (pc 0x7fbe1ee95c2b bp 0x7fff7fb41610 sp 0x7fff7fb40d70 T0)"
9: B1 Line exceeds max length (156>80): "    #0 0x7fbe1ee95c2b in printf_common(void*, char const*, __va_list_tag*) (/lib64/libasan.so.8+0x95c2b) (BuildId: 5395ec74f54d9ec7bf97c06583dd39a96c230822)"
10: B1 Line exceeds max length (116>80): "    #1 0x7fbe1eeb72d6 in vsnprintf (/lib64/libasan.so.8+0xb72d6) (BuildId: 5395ec74f54d9ec7bf97c06583dd39a96c230822)"
11: B1 Line exceeds max length (115>80): "    #2 0x7fbe1eeb94f4 in snprintf (/lib64/libasan.so.8+0xb94f4) (BuildId: 5395ec74f54d9ec7bf97c06583dd39a96c230822)"
12: B1 Line exceeds max length (101>80): "    #3 0x000000401fc2 in convert_raw_data_to_xml ../../../../Projects/jhbuild/bluez/src/sdp-xml.c:709"
13: B1 Line exceeds max length (101>80): "    #4 0x000000401c4f in convert_raw_data_to_xml ../../../../Projects/jhbuild/bluez/src/sdp-xml.c:994"
15: B1 Line exceeds max length (103>80): "    #246 0x000000401c4f in convert_raw_data_to_xml ../../../../Projects/jhbuild/bluez/src/sdp-xml.c:994"
17: B1 Line exceeds max length (121>80): "SUMMARY: AddressSanitizer: stack-overflow ../../../../Projects/jhbuild/bluez/src/sdp-xml.c:709 in convert_raw_data_to_xml"
##############################
Test: MakeDistcheck - FAIL
Desc: Run Bluez Make Distcheck
Output:

make[4]: *** [Makefile:10329: test-suite.log] Error 1
make[3]: *** [Makefile:10437: check-TESTS] Error 2
make[2]: *** [Makefile:10915: check-am] Error 2
make[1]: *** [Makefile:10917: check] Error 2
make: *** [Makefile:10838: distcheck] Error 1


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

---
Regards,
Linux Bluetooth