Fix heap-buffer-overflow in sdp_xml.c:compute_seq_size

This is a follow up to my first patch attempt. After scrutinizing this a bit more, it turns out my previous patch wasn't actually addresing the root cause.

The ASan stacktrace (found by OSS-Fuzz) for this issue is:

```
==399==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x502000000218 at pc 0x5cffda946877 bp 0x7ffe90702810 sp 0x7ffe90702808
READ of size 4 at 0x502000000218 thread T0
    #0 0x5cffda946876 in compute_seq_size bluez/src/sdp-xml.c:62:21
    #1 0x5cffda946876 in element_end bluez/src/sdp-xml.c:548:42
    #2 0x5cffda984416 in emit_end_element glib/glib/gmarkup.c:1045:5
    #3 0x5cffda983978 in g_markup_parse_context_parse glib/glib/gmarkup.c:1603:19
    #4 0x5cffda944c55 in sdp_xml_parse_record bluez/src/sdp-xml.c:621:6
    #5 0x5cffda949cf1 in LLVMFuzzerTestOneInput /src/fuzz_xml.c:30:9
    #6 0x5cffda7f9730 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:614:13
    #7 0x5cffda7e49a5 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:327:6
    #8 0x5cffda7ea43f in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:862:9
    #9 0x5cffda8156e2 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
    #10 0x7e0ccb446082 in __libc_start_main /build/glibc-LcI20x/glibc-2.31/csu/libc-start.c:308:16
    #11 0x5cffda7dcb8d in _start
```

This patch should address this issue properly. Please see the commit message for more details.

Oliver Chang (1):
Fix heap-buffer-overflow in sdp_xml.c:compute_seq_size

 src/sdp-xml.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

2.51.0.rc0.205.g4a044479a3-goog

https://issues.oss-fuzz.com/issues/42516062
https://oss-fuzz.com/testcase-detail/5896441415729152

This can be triggered by using an input of <sequence><foo/><text/></sequence> against the harness in
https://github.com/google/oss-fuzz/blob/master/projects/bluez/fuzz_xml.c

The root cause of the heap-buffer-overflow was incorrect stack management in the SDP XML parser (element_end function) that led to type confusion.

When an XML element failed to parse (e.g., an unrecognized tag like <foo/>), its corresponding entry was left on the parser stack because the we returned early if data was NULL.

With the input <sequence><foo/><text/></sequence>, <foo/> failed parsing and remained on the stack with a NULL data. Then <text/> was parsed and also remained on the stack because it's only popped if ctx_data->stack_head->next->data != NULL.

When </sequence> was encountered, the parser then mistakenly used the data from <text/> (which was now at the top of the stack) as the sequence data. This led to a type confusion: the TEXT data's string pointer (val.str) was interpreted as a sequence pointer (val.dataseq). This pointer pointed to a 1-byte allocation (for the empty string). The code then tried to dereference this pointer as an sdp_data_t struct to calculate the sequence size, leading to the out-of-bounds read.

To fix this, in element_end, ensure that the stack is popped even if the element's data failed to parse. This prevents the stack desynchronization.

---
 src/sdp-xml.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

index a83dec157..e5b30e885 100644
@@ -545,8 +545,15 @@ static void element_end(GMarkupParseContext *context,
 		return;
 	}
 
-	if (!ctx_data->stack_head || !ctx_data->stack_head->data) {
+	if (!ctx_data->stack_head)
+		return;
+
+	if (!ctx_data->stack_head->data) {
 		DBG("No data for %s", element_name);
+
+		elem = ctx_data->stack_head;
+		ctx_data->stack_head = ctx_data->stack_head->next;
+		sdp_xml_data_free(elem);
 		return;
 	}
 

2.51.0.rc0.205.g4a044479a3-goog

To clarify which "previous patch" I was referring to, I'm referring to the one titled "[PATCH BlueZ 1/1] Fixed heap-buffer-overflow in compute_seq_size"

Kind regards,
Oliver

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=990970

---Test result---

Test Summary:

CheckPatch                    PENDING   0.29 seconds
GitLint                       PENDING   0.26 seconds
BuildEll                      PASS      20.00 seconds
BluezMake                     PASS      2559.06 seconds
MakeCheck                     PASS      20.01 seconds
MakeDistcheck                 PASS      185.63 seconds
CheckValgrind                 PASS      237.11 seconds
CheckSmatch                   PASS      306.15 seconds
bluezmakeextell               PASS      128.33 seconds
IncrementalBuild              PENDING   0.32 seconds
ScanBuild                     PASS      911.91 seconds

Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:

##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:

##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series Output:

---
Regards,
Linux Bluetooth

Hello,

I just wanted to check if there was any feedback on my patch?

Please let me know if I can help clarify any details.

Kind regards,
Oliver