Re: [PATCH BlueZ 1/1] Fixed heap-buffer-overflow in `compute_seq_size`.

[Bastien Nocera <hadess@xxxxxxxxxx>]

On Fri, 2025-08-08 at 01:34 +0000, Oliver Chang wrote:
> By adding checks for sequence/alternate types in element_end and
> ensuring proper cleanup in sdp_xml_parse_record error path.
> 
> https://issues.oss-fuzz.com/issues/42516062
> https://oss-fuzz.com/testcase-detail/5896441415729152

Neither of those URLs show any details about the problem that was
detected.

It would be useful if there was a regression test in unit/ either in
unit/sdp.c or as a separate sdp-xml.c test that, for example, showed
the lost memory in a valgrind run, which the patch would fix.

Cheers

> ---
>  src/sdp-xml.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/src/sdp-xml.c b/src/sdp-xml.c
> index 5efa62ab8..b10882db2 100644
> --- a/src/sdp-xml.c
> +++ b/src/sdp-xml.c
> @@ -545,6 +545,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,
> +					"Unexpected data type %d for
> sequence",
> +					ctx_data->stack_head->data-
> >dtd);
> +			return;
> +		}
> +
>  		ctx_data->stack_head->data->unitSize =
> compute_seq_size(ctx_data->stack_head->data);
>  
>  		if (ctx_data->stack_head->data->unitSize >
> USHRT_MAX) {
> @@ -557,6 +564,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,
> +					"Unexpected data type %d for
> alternate",
> +					ctx_data->stack_head->data-
> >dtd);
> +			return;
> +		}
> +
>  		ctx_data->stack_head->data->unitSize =
> compute_seq_size(ctx_data->stack_head->data);
>  
>  		if (ctx_data->stack_head->data->unitSize >
> USHRT_MAX) {
> @@ -621,6 +635,11 @@ sdp_record_t *sdp_xml_parse_record(const char
> *data, int size)
>  	if (g_markup_parse_context_parse(ctx, data, size, NULL) ==
> FALSE) {
>  		error("XML parsing error");
>  		g_markup_parse_context_free(ctx);
> +		while (ctx_data->stack_head) {
> +			struct sdp_xml_data *elem = ctx_data-
> >stack_head;
> +			ctx_data->stack_head = elem->next;
> +			sdp_xml_data_free(elem);
> +		}
>  		sdp_record_free(record);
>  		free(ctx_data);
>  		return NULL;