Re: [PATCH v2] usb: gadget: rndis: Prevent InformationBufferOffset manipulation

[Marek Vasut <marex@denx.de>]

On 12/5/22 10:28, Szymon Heidrich wrote:
> Prevent access to arbitrary memory locations in gen_ndis_set_resp
> via manipulation of buf->InformationBufferOffset. Original
> implementation permits manipulation of InformationBufferOffset to
> exploit OID_GEN_CURRENT_PACKET_FILTER to set arbitrary memory contents
> within a 32byte offset as the devices packet filter. The packet filter
> value may be next retrieved using gen_ndis_query_resp so it is possible
> to extract specific memory regions two bytes a time.
> 
> The rndis_query_response was not modified as neither the buffer offset
> nor length passed to gen_ndis_query_resp is used.
> 
> Signed-off-by: Szymon Heidrich <szymon.heidrich@gmail.com>
> ---
> V1 -> V2: Updated commit message
> 
>   drivers/usb/gadget/rndis.c | 9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/gadget/rndis.c b/drivers/usb/gadget/rndis.c
> index 13c327ea38..3948f2cc9a 100644
> --- a/drivers/usb/gadget/rndis.c
> +++ b/drivers/usb/gadget/rndis.c
> @@ -855,14 +855,17 @@ static int rndis_set_response(int configNr, rndis_set_msg_type *buf)
>   	rndis_set_cmplt_type	*resp;
>   	rndis_resp_t		*r;
>   
> +	BufLength = get_unaligned_le32(&buf->InformationBufferLength);
> +	BufOffset = get_unaligned_le32(&buf->InformationBufferOffset);
> +	if ((BufOffset > RNDIS_MAX_TOTAL_SIZE - 8) ||
> +	    (BufLength > RNDIS_MAX_TOTAL_SIZE - 8 - BufOffset))
> +		return -EINVAL;
> +
>   	r = rndis_add_response(configNr, sizeof(rndis_set_cmplt_type));
>   	if (!r)
>   		return -ENOMEM;
>   	resp = (rndis_set_cmplt_type *) r->buf;
>   
> -	BufLength = get_unaligned_le32(&buf->InformationBufferLength);
> -	BufOffset = get_unaligned_le32(&buf->InformationBufferOffset);
> -
>   #ifdef	VERBOSE
>   	debug("%s: Length: %d\n", __func__, BufLength);
>   	debug("%s: Offset: %d\n", __func__, BufOffset);

Applied to usb/master, thanks