PDA

Bekijk Volledige Versie : Linux Kernel i2c Integer Overflow Vulnerability



Shaun Colley
17/06/04, 03:15
__________________________________________________ _______

Product: Linux Kernel
i2c driver
Versions: 2.4.x

Bug: Integer Overflow
Impact: Kernel Panic
Code execution with kernel privs
Risk: High/Medium
Date: June 16, 2004
Author: shaun2k2
http://www.nettwerked.co.uk
__________________________________________________ _______




Introduction
#############

The Linux Kernel is the core of the Linux Operating
System, and provides the usual features of a modern
multi-user kernel. Drivers to support many different
devices are available packaged with the Linux Kernel,
including the 'i2c' driver, which provides support for
the 2-wire I2C bus.

In the i2c driver, there is an integer overflow
vulnerability during the allocation of memory,
potentially opening any systems using the i2c driver
up to a security hole.



Details
########

The integer overflow problem becomes present when
allocating memory, which is allowed to occur because
of a lack of sanity checks. Below is the vulnerable
code, which is part of the i2cproc_bus_read() routine,
in the i2c-core.c component of the driver.

--- 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:

---
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

Alexander Nyberg
18/06/04, 03:55
> --- 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

Greg KH
19/06/04, 09:25
On Thu, Jun 17, 2004 at 12:51:01PM +0100, Shaun Colley wrote:
> 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:

<snip>

Yes, this was a bug, 11 months ago, all distros have had this fixed in
their latest kernels for a long time now. There was previous
discussions about this on the linux-kernel mailing list, so you weren't
the first to find it by a long shot.

Also, distros ship their /dev/i2c* nodes as accessable only as root, so
again, this isn't really a problem.

thanks,

greg k-h

Shaun Colley
20/06/04, 10:26
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