Re: Linux Kernel i2c Integer Overflow Vulnerability

[Shaun Colley <shaunige () yahoo co uk>]

Well, okay, I appreciate all emails I got about my
error.  As you've all pointed out, this function is
safe so forget the misinformation.  However, there is
a vulnerability in the i2c ioctl() code, which exists
because of a possible integer overflow.  I did discuss
this on the LKML with Greg Kroah-Hartman, who I
believe has a bit to do with the i2c driver.  Please
get off my back about this stupid mistake, I'm putting
it right now...


Okay, non-"fake" stuff:




~*~*~*~*~*~*~*~*~*~*~*~*~*~*~*~*~*~*~*~*~*~*~*~*~*~*~*~*

Product:      Linux Kernel
              i2c driver
Versions:     <= 2.4.20
              2.5.x
Bug:          Integer overflow
Impact:       Kernel panic
              Code execution with kernel privs.
Risk:         High/Medium
Date:         June 17, 2004
Author:       Shaun Colley
              Email: shaunige yahoo co uk
              WWW: http://www.nettwerked.co.uk

~*~*~*~*~*~*~*~*~*~*~*~*~*~*~*~*~*~*~*~*~*~*~*~*~*~*~*~*



Details
########

There is a potential integer overflow which can occur
during the allocation of memory, during parsing of the
I2C_RDWR option in the i2cdev_ioctl() routine.  Below
is the vulnerable code:

---
case I2C_RDWR:
if (copy_from_user(&rdwr_arg,
(struct
i2c_rdwr_ioctl_data *)arg,
sizeof(rdwr_arg)))
return -EFAULT;

rdwr_pa = (struct i2c_msg *)
kmalloc(rdwr_arg.nmsgs *
sizeof(struct i2c_msg),
GFP_KERNEL);

if (rdwr_pa == NULL) return -ENOMEM;
---

Due to lack of sanity checks, an integer overflow
could occur because of the arithmetic operation
(rdwr_arg.nmsgs * sizeof(struct i2c_msg)) used in the
kmalloc() call.

If a user supplied a large integer to be used in the
arithmetic operation in the length argument on the
kmalloc() call, an integer overflow could occur,
resulting in too little memory being allocated.  Below
is the vulnerable kmalloc() call:

---
kmalloc(rdwr_arg.nmsgs * sizeof(struct i2c_msg),
GFP_KERNEL);
---

Since the integer held by rdwr_arg.nmsgs is
user-supplied, a suitably calculated integer could
cause the integer overflow, when multiplied by 'sizeof
struct i2c_msg'.

Following the kmalloc() call, a for() loop is used in
conjuction with copy_from_user to occupy the newly
allocated memory chunk.  However, since a user
could've potentially caused an integer overflow to
occur, user-supplied data may be written beyond the
bounds of the too-small memory chunk.  Below are the
following copy calls:

---
for( i=0; i<rdwr_arg.nmsgs; i++ )
                 {
                        
if(copy_from_user(&(rdwr_pa[i]),
                                        
&(rdwr_arg.msgs[i]),
                                        
sizeof(rdwr_pa[i])))
                         {
                                 res = -EFAULT;
                                 break;
                         }
                         rdwr_pa[i].buf =
kmalloc(rdwr_pa[i].len, GFP_KERNEL);
                         if(rdwr_pa[i].buf == NULL)
                         {
                                 res = -ENOMEM;
                                 break;
                         }
                        
if(copy_from_user(rdwr_pa[i].buf,
                                 rdwr_arg.msgs[i].buf,
                                 rdwr_pa[i].len))
                         {

[...]
---

It is apparent that there is a possiblity of writing
past the allocated memory bounds, as a result of the
integer overflow.

This bug is present in the i2c driver packaged with
kernels 2.4.20 and earlier 2.4 kernels.  Also, this
seems to be present in the latest 2.5 source tree.  It
is fixed in 2.6 and 2.4.21+

Apparently, mentioned by Greg Kroah-Hartman, 'most'
Linux distributions set 0600 permissions on the
/dev/i2c-* nodes, so the chance of exploitation in
some systems might be slim.  Once again, this is only
an issue if the i2c driver is installed on the system.
 


Solution
#########

Upgrade to 2.4.21 (which is pretty old to start with).
 The latest version of 2.5 is vulnerable, but I
understand devel is dead.  Some sanity checks like
these could be added to the I2C_RDWR option of the
i2cdev_ioctl() function, in i2c-dev.c:

---
[...]

if(rdwr_arg.nmsgs > 42)
         rdwr_arg.nmsgs = 42;

if(rdwr_arg.nmsgs < 0)
         rdwr_arg.nmsgs = 0;

[...]
---

All recent versions of 2.4.x are safe, so that should
be your best bet. 




Now that I've attempted to clear it up (my fuckup),
please don't continue to harp on about it...:)

[1]
http://www.uwsg.iu.edu/hypermail/linux/kernel/0406.1/1059.html
- discussion of the bugs.




Thank you for your time.
Shaun.


        
        
                
___________________________________________________________ALL-NEW Yahoo! Messenger - sooooo many all-new ways to 
express yourself http://uk.messenger.yahoo.com