> --- vuln code --- > ssize_t i2cproc_bus_read(struct file * file, char * > buf,size_t count, > loff_t *ppos) > { > struct inode * inode = > file->f_dentry->d_inode; > char *kbuf; > struct i2c_client *client; > int i,j,k,order_nr,len=0; > size_t len_total; > int order[I2C_CLIENT_MAX]; > > if (count > 4000) > return -EINVAL; > len_total = file->f_pos + count; > /* Too bad if this gets longer (unlikely) */ > if (len_total > 4000) > len_total = 4000; > for (i = 0; i < I2C_ADAP_MAX; i++) > if (adapters[i]->inode == > inode->i_ino) { > /* We need a bit of slack in the > kernel buffer; this makes the > sprintf safe. */ > if (! (kbuf = kmalloc(count + > 80,GFP_KERNEL))) > return -ENOMEM; > > [...] > > --- end snippet --- > > Although a quick check is made to ensure that the > user-supplied variable 'count' does not exceed 4000, > sanity checks do not occur to check for negative > integers in 'count'. Since negative integers simply > become _very_ large integers when represented as > unsigned, a negative count argument to kmalloc() would > cause unexpected behavior: > On i386: typedef __kernel_size_t size_t; typedef unsigned int __kernel_size_t; asmlinkage ssize_t sys_read(unsigned int fd, char * buf, size_t count); ^^^^^^^ You can't pass negative values, all archs have unsigned definitions of size_t. Therefor "if (count > 4000)" is an ok range check. > --- > if (! (kbuf = kmalloc(count + 80,GFP_KERNEL))) > --- > > For example, if '-1' was passed to the routine as the > 'count' argument, the above kmalloc() call would be > equivalent to below: > > --- > if (! (kbuf = kmalloc(0xffffffff + 80,GFP_KERNEL))) > --- > > This would cause an integer overflow during the > kmalloc() call when 80 is added to count, resulting in > a very small amount of memory being allocated. > > As in the comment just above the vulnerable kmalloc() > call (/* We need a bit of slack in the kernel buffer; > this makes the sprintf safe. */), the purpose of > incrementing the 'count' argument by 80 is to stop the > chance of a buffer overflow, but by supplying a > suitable negative integer as 'count' (i.e -1), this > allows an integer overflow, causing the kmalloc() > argument to wrap back round to a small/negative value. > > In the sprintf() calls following the kmalloc() call, > there is quite a possibility of overflowing the bounds > of the newly allocated very small chunk of memory. > This might result in kernel panic, corruption of > kernel memory, or maybe even elevation of privileges > (*very* unlikely). > > i2cproc_bus_read() is implemented as a read() hook in > the driver, as below: > > --- > static struct file_operations i2cproc_operations = { > read: i2cproc_bus_read, > }; > --- > > This might allow unprivileged users to exploit the > issue. > > Please take note that this potential security hole > only affects those using the i2c driver -- if this > driver (it can be installed as either a module or > built into the kernel) is not installed on your > system, you're not vulnerable. The issue is present > in all 2.4 kernels, including the latest release. > > > > Solution > ######### > > The following sanity check can be added to the > beginning of the i2cproc_bus_read() in the i2c-core.c > file: > > --- > if(count < 0) > return -EINVAL; > --- > > Then rebuild the kernel, and the issue should be > resolved. > > A possible workaround would be to perhaps disable the > module or remove the driver if it's not needed on your > system. > > > > Disclaimer > ########### > > The information contained within this advisory was > believed to be accurate at the time of it's > publishing. However, it might be inaccurate at times, > so don't consider any information contained within > definitely correct. > > Direct flames to /dev/null. Don't bothering wasting > your time and mine with any crap about any disclosure > policies I may or may not have followed -- I'm not > interested, so I'll just ignore you if you don't > phrase things nicely. > > > > Thank you for your time. > Shaun. > > > > > > ___________________________________________________________ALL-NEW Yahoo! Messenger - sooooo many all-new ways to express yourself http://uk.messenger.yahoo.com