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

[Paul Menzel <pmenzel@xxxxxxxxxxxxx>]

Dear Oliver,


Thank you for the patch. For the summary, I’d use imperative mood and do 
not add a dot/period at the end:

Fix heap-buffer-overflow in `compute_seq_size`

Am 10.08.25 um 08:46 schrieb Oliver Chang:
> By adding checks for sequence/alternate types in element_end to avoid a
> type confusion.
>
> This issue was found by OSS-Fuzz.
>
> 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
>
> https://issues.oss-fuzz.com/issues/42516062

The last comment says:

> This issue was migrated from crbug.com/oss-fuzz/51480?no_tracker_redirect=1

But that URL gives *Page Not Found*.

> https://oss-fuzz.com/testcase-detail/5896441415729152

I am unable to access this without logging in.

With your patch and the test case, what error is logged now?

> ---
>   src/sdp-xml.c | 14 ++++++++++++++
>   1 file changed, 14 insertions(+)
>
> diff --git a/src/sdp-xml.c b/src/sdp-xml.c
> index 5efa62ab8..81bd11140 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) {


Thank you again and kind regards,

Paul