lkml.org 
[lkml]   [2015]   [May]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH 1/4] ozwpan: Use proper check to prevent heap overflow
Date
From: Jason A. Donenfeld
> Sent: 13 May 2015 19:34
> Since elt->length is a u8, we can make this variable a u8. Then we can
> do proper bounds checking more easily. Without this, a potentially
> negative value is passed to the memcpy inside oz_hcd_get_desc_cnf,
> resulting in a remotely exploitable heap overflow with network
> supplied data.
...
> diff --git a/drivers/staging/ozwpan/ozusbsvc1.c b/drivers/staging/ozwpan/ozusbsvc1.c
> index d434d8c..cd6c63e 100644
> --- a/drivers/staging/ozwpan/ozusbsvc1.c
> +++ b/drivers/staging/ozwpan/ozusbsvc1.c
> @@ -390,8 +390,10 @@ void oz_usb_rx(struct oz_pd *pd, struct oz_elt *elt)
> case OZ_GET_DESC_RSP: {
> struct oz_get_desc_rsp *body =
> (struct oz_get_desc_rsp *)usb_hdr;
> - int data_len = elt->length -
> + u8 data_len = elt->length -
> sizeof(struct oz_get_desc_rsp) + 1;
> + if (data_len > elt->length)
> + break;

Why not just check the length. eg:
unsigned int data_len = elt->length;
if (data_len < sizeof(struct oz_get_desc_rsp) + 1)
break;

> u16 offs = le16_to_cpu(get_unaligned(&body->offset));
> u16 total_size =
> le16_to_cpu(get_unaligned(&body->total_size));

Don't put variable definitions after code.

You don't really want to do arithmetic on local variables that are
smaller than a machine word (eg u8 and u16), doing so can require
the compiler generate a lot more code.

David



\
 
 \ /
  Last update: 2015-05-15 19:01    [W:0.088 / U:0.548 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site