[Spice-devel] [PATCH usbredir v2.1] usbredirserver: reject empty vendorid in cmd line

[Chen Hanxiao]

At 2017-11-28 21:16:15, "Frediano Ziglio" <fziglio at redhat.com> wrote:
>> 
>> From: Chen Hanxiao <chenhanxiao at gmail.com>
>> 
>> Vendor ID 0000 is not a valid ID [1]
>> But we could pass it from cmd:
>>   usbredirserver :abcd
>> Which will cause a 0000 vendor id.
>> 
>> This patch will check this senario.
>> 
>> [1]: http://www.linux-usb.org/usb.ids
>> 
>> Signed-off-by: Chen Hanxiao <chenhanxiao at gmail.com>
>> ---
>> v2.1:
>>   fix a copy-paste error
>> v2:
>>   add range check of vid/pid
>> 
>>  usbredirserver/usbredirserver.c | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>> 
>> diff --git a/usbredirserver/usbredirserver.c
>> b/usbredirserver/usbredirserver.c
>> index 5a4adc5..56f91ec 100644
>> --- a/usbredirserver/usbredirserver.c
>> +++ b/usbredirserver/usbredirserver.c
>> @@ -259,11 +259,13 @@ int main(int argc, char *argv[])
>>              invalid_usb_device_id(argv[optind], argv[0]);
>>          }
>>          usbvendor = strtol(argv[optind], &endptr, 16);
>> -        if (*endptr != ':') {
>> +        if (*endptr != ':' || usbvendor <= 0
>> +            || usbvendor > 0xffff) {
>>              invalid_usb_device_id(argv[optind], argv[0]);
>>          }
>>          usbproduct = strtol(delim + 1, &endptr, 16);
>> -        if (*endptr != '\0') {
>> +        if (*endptr != '\0' || usbproduct < 0
>> +            || usbproduct > 0xffff) {
>>              invalid_usb_device_id(argv[optind], argv[0]);
>>          }
>>      }
>
>All other ifs in the code are split like
>
>       if (*endptr != '\0' || usbproduct < 0 ||
>           usbproduct > 0xffff) {
>
>not
>
>       if (*endptr != '\0' || usbproduct < 0
>           || usbproduct > 0xffff) {
>
>(looks really consistent). I honestly don't think
>
>       if (*endptr != '\0' || usbproduct < 0 || usbproduct > 0xffff) {
>
>is too long.

Thanks for the review.
I'll fix it in v3

Regards,
- Chen