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

[Szymon Heidrich <szymon.heidrich@gmail.com>]

On 09/12/2022 02:56, Marek Vasut wrote:
> 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

Thank you very much for your time and review.