Re: [PATCH BlueZ v1 1/5] avrcp: Fix out-of-bounds parsing of ListPlayerAttributes response

[Bastien Nocera <hadess@hadess.net>]

On Tue, 2026-09-01 at 13:53 -0400, Luiz Augusto von Dentz wrote:
> From: Bastien Nocera <hadess@hadess.net>

The patch has substantially changed from the version I posted
privately, so you can remove my authorship.

> In profiles/audio/avrcp.c, avrcp_list_player_attributes_rsp() parsed
> the
> response using hand-computed offsets into the operands buffer,
> without
> accounting for the fact that operand_count spans the 7 byte AVRCP
> header
> as well as the parameters:
> 
> - attrs is a 4 byte array which could be written out-of-bounds if a
>   length greater than 4 was declared in the first parameter byte.
> 
> - The attribute bytes were read with a bound derived from
> operand_count,
>   so a truncated response could be read past its end. As the receive
>   buffer is reused across packets, those stale bytes could be echoed
>   back to the peer in the following GetCurrentPlayerValue request.
> 
> - params_len was compared against count, which was only ever 0 at
> that
>   point, so the length of the PDU was in practice never validated.
> 
> Parse the response through a struct iovec using the util_iov_pull_*
> helpers instead, so that the header and each subsequent field are
> bounds
> checked as they are consumed and the remaining length is tracked for
> us.
> This lets params_len be validated against the actual number of
> parameter
> bytes received. The attribute count is still clamped to
> AVRCP_ATTRIBUTE_LAST, which is what bounds the write into attrs.
> 
> Reported-by: @ax-nnlabs
> Closes:
> https://github.com/bluez/bluez/security/advisories/GHSA-m2vx-pw5f-rc8v

It was embargoed, and it's now been made public.

> ---
>  profiles/audio/avrcp.c | 32 +++++++++++++++++++++++++-------
>  1 file changed, 25 insertions(+), 7 deletions(-)
> 
> diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
> index 3271b84782e8..906f93424872 100644
> --- a/profiles/audio/avrcp.c
> +++ b/profiles/audio/avrcp.c
> @@ -2395,31 +2395,49 @@ static gboolean
> avrcp_list_player_attributes_rsp(struct avctp *conn,
>  					uint8_t transaction, uint8_t
> *operands,
>  					size_t operand_count, void
> *user_data)
>  {
> +	struct iovec iov = { operands, operand_count };
>  	uint8_t attrs[AVRCP_ATTRIBUTE_LAST];
>  	struct avrcp *session = user_data;
> -	struct avrcp_header *pdu = (void *) operands;
> +	struct avrcp_header *pdu;
>  	uint8_t len, count = 0;
>  	int i;
>  
>  	if (code == AVC_CTYPE_REJECTED || code ==
> AVC_CTYPE_NOT_IMPLEMENTED)
>  		return FALSE;
>  
> -	len = pdu->params[0];
> +	pdu = util_iov_pull_mem(&iov, sizeof(*pdu));
> +	if (!pdu) {
> +		error("Invalid AVRCP header");
> +		return FALSE;
> +	}
>  
> -	if (be16_to_cpu(pdu->params_len) < count) {
> +	if (be16_to_cpu(pdu->params_len) != iov.iov_len) {
>  		error("Invalid parameters");
>  		return FALSE;
>  	}
>  
> -	for (i = 0; len > 0; len--, i++) {
> +	if (!util_iov_pull_u8(&iov, &len))
> +		return FALSE;
> +
> +	len = MIN(len, AVRCP_ATTRIBUTE_LAST);
> +
> +	for (i = 0; i < len; i++) {
> +		uint8_t attr;
> +
> +		if (!util_iov_pull_u8(&iov, &attr))
> +			break;
> +
>  		/* Don't query invalid attributes */
> -		if (pdu->params[i + 1] == AVRCP_ATTRIBUTE_ILLEGAL ||
> -				pdu->params[i + 1] >
> AVRCP_ATTRIBUTE_LAST)
> +		if (attr == AVRCP_ATTRIBUTE_ILLEGAL ||
> +					attr > AVRCP_ATTRIBUTE_LAST)
>  			continue;
>  
> -		attrs[count++] = pdu->params[i + 1];
> +		attrs[count++] = attr;
>  	}
>  
> +	if (!count)
> +		return FALSE;
> +
>  	avrcp_get_current_player_value(session, attrs, count);
>  
>  	return FALSE;