[BlueZ, v4 2/9] sdp-xml: Fix crash caused by type confusion when parsing crafted SDP XML
Bastien Nocera <[email protected]>
| Newsgroups | org.kernel.vger.linux-bluetooth |
|---|---|
| Message-ID | <[email protected]> |
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