Staging: vt6655/vt6656 security issues

Staging: vt6655/vt6656 security issues

[Dan Rosenberg]

Based on a brief glance looking for security issues, I just wanted to
mention that these drivers are nowhere near ready to be added to the
main kernel.  I'm not interested in developing these drivers further,
but there are at least six stack buffer overflows:

vt6655/wpactl.c: wpa_set_keys(), line 239
vt6655/wpactl.c: wpa_set_keys(), line 280
vt6655/wpactl.c: wpa_set_associate(), line 770 (reported by Dan
Carpenter)

vt6656/wpactl.c: wpa_set_keys(), line 239
vt6656/wpactl.c: wpa_set_keys(), line 279
vt6656/wpactl.c: wpa_set_associate(), line 779

And four heap corruption issues due to integer overflow in the
allocation size:

vt6655/ioctl.c: private_ioctl(), line 329
vt6655/ioctl.c: private_ioctl(), line 625

vt6656/ioctl.c: private_ioctl(), line 326
vt6656/ioctl.c: private_ioctl(), line 615

They are all caused by unchecked copy_from_user() calls with
user-provided length fields or kmalloc() calls with arithmetic on
user-provided sizes.  This kind of sloppiness suggests there are almost
certainly other major security issues in this code.

Regards,
Dan

Re: Staging: vt6655/vt6656 security issues

[Greg KH]

On Mon, Sep 27, 2010 at 02:30:56PM -0400, Dan Rosenberg wrote:
> Based on a brief glance looking for security issues, I just wanted to
> mention that these drivers are nowhere near ready to be added to the
> main kernel.  I'm not interested in developing these drivers further,
> but there are at least six stack buffer overflows:
> 
> vt6655/wpactl.c: wpa_set_keys(), line 239
> vt6655/wpactl.c: wpa_set_keys(), line 280
> vt6655/wpactl.c: wpa_set_associate(), line 770 (reported by Dan
> Carpenter)
> 
> vt6656/wpactl.c: wpa_set_keys(), line 239
> vt6656/wpactl.c: wpa_set_keys(), line 279
> vt6656/wpactl.c: wpa_set_associate(), line 779
> 
> And four heap corruption issues due to integer overflow in the
> allocation size:
> 
> vt6655/ioctl.c: private_ioctl(), line 329
> vt6655/ioctl.c: private_ioctl(), line 625
> 
> vt6656/ioctl.c: private_ioctl(), line 326
> vt6656/ioctl.c: private_ioctl(), line 615
> 
> They are all caused by unchecked copy_from_user() calls with
> user-provided length fields or kmalloc() calls with arithmetic on
> user-provided sizes.  This kind of sloppiness suggests there are almost
> certainly other major security issues in this code.

I would not doubt that at all.  For the moment, these drivers are being
used to allow users to use their machines, and a "real" driver is soon
replacing them for the long-term.  They also all run on laptops, where
such security issues are not as relevant due to the "physical access"
mode.  Not that this excuses such horrible behavior, but it does lesten
the threat model a lot.

I thank you for your patches and looking into this code, but if you
want, don't really worry about future work in this area, as the drivers
will just be deleted entirely soon (with any luck).

thanks,

greg k-h