Re: [PATCH] Enforce buffer boundaries on RNDIS USB Gadget

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

On 20/11/2022 16:02, Fabio Estevam wrote:
> Szymon,
> 
> On Thu, Nov 17, 2022 at 4:46 PM Szymon Heidrich
> <szymon.heidrich@gmail.com> wrote:
>>
>> Prevent access to arbitrary memory locations in gen_ndis_set_resp
>> via manipulation of buf->InformationBufferOffset. Lack of validation
>> of BufOffset could be exploited to dump arbitrary memory contents
>> via NDIS packet filter.
>>
>> Signed-off-by: Szymon Heidrich <szymon.heidrich@gmail.com>
> 
> Please run ./scripts/get_maintainer.pl on your patch and copy the maintainers.
> 

Hello Fabio,

Sorry I missed adding Lukasz and Marek - I'll keep that in mind for future.

Is there anything else missing from my side?

Best regards,
Szymon

> 
>> ---
>>  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);
>> --
>> 2.38.1
>>