[Spice-devel] [PATCH usbredir] usbredirserver: reject empty vendor id in cmd line

[Frediano Ziglio]

> > 
> > At 2017-11-28 18:27:54, "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
> > >>    or
> > >>   usbredirserver 0000:abcd
> > >> 
> > >> Which will pass a 0000 vendor id to usbredirserver.
> > >> 
> > >> This patch will check this senario.
> > >> 
> > >> [1]: http://www.linux-usb.org/usb.ids
> > >> 
> > >> Signed-off-by: Chen Hanxiao <chenhanxiao at gmail.com>
> > >> ---
> > >>  usbredirserver/usbredirserver.c | 2 +-
> > >>  1 file changed, 1 insertion(+), 1 deletion(-)
> > >> 
> > >> diff --git a/usbredirserver/usbredirserver.c
> > >> b/usbredirserver/usbredirserver.c
> > >> index 5a4adc5..17226a5 100644
> > >> --- a/usbredirserver/usbredirserver.c
> > >> +++ b/usbredirserver/usbredirserver.c
> > >> @@ -259,7 +259,7 @@ 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) {
> > >>              invalid_usb_device_id(argv[optind], argv[0]);
> > >>          }
> > >>          usbproduct = strtol(delim + 1, &endptr, 16);
> > >
> > >Maybe you want something like
> > >
> > >    if (*endptr != ':' || usbvendor <= 0 || usbvendor > 0xffff) {
> > >
> > >similar for usbproduct.
> > 
> > We don't need to check for usbvendor <0, for we don't have long options for
> > usbvendor:usbproduct.
> > So I don't know how to pass a negative value to it.
> > 
> 
> Well, try "81234567:-123" :-)
> 

Another way to get a negative number without using overflow trick is
to pass a string like " -123" to strtol, the space avoids to be parsed
as an option and will be ignored by strtol.
Strangely somebody may be tempted by fixing this using strtoul but
for some reasons strtoul accepts the sign, even negative!

> > 
> > The check for <= 0xffff looks reasonable.
> > 
> > Regards,
> > - Chen