xorl %eax, %eax

Linux kernel nvram Integer Overflow

with 5 comments

Keeping my promise to spender, here is an interesting email I just saw in lkml by Michael Buesch. So.. nvram is a classic CMOS/NV-RAM device driver for Linux which can be found at drivers/char/nvram.c. M. Buesch noticed (and also gave a nice PoC code) that the following routine:

static ssize_t nvram_write(struct file *file, const char __user *buf,
size_t count, loff_t *ppos)
{
unsigned char contents[NVRAM_BYTES];
unsigned i = *ppos;
unsigned char *tmp;
int len;

len = (NVRAM_BYTES – i) < count ? (NVRAM_BYTES - i) : count; if (copy_from_user(contents, buf, len)) return -EFAULT; spin_lock_irq(&rtc_lock); if (!__nvram_check_checksum()) goto checksum_err; for (tmp = contents; count-- > 0 && i < NVRAM_BYTES; ++i, ++tmp) __nvram_write_byte(*tmp, i); __nvram_set_checksum(); spin_unlock_irq(&rtc_lock); *ppos = i; return tmp - contents; checksum_err: spin_unlock_irq(&rtc_lock); return -EIO; } [/sourcecode] Can lead to a classic (thanks to Silvio) integer overflow during len calculation. Specifically, len is defined as a signed integer (which I don’t see the reason why) and then it calculates its legnth using:
(NVRAM_BYTES – i) < count ? (NVRAM_BYTES – i) : count
Variable 'i' represents the file offset as you can see from the function's scope where it is defined as loff_t *ppos. Also, count is 100% user controlled variable that represents the requested size in bytes. Since no check is being performed during NVRAM_BYTES – i substration an attacker can overflow 'i' and copy 'count' bytes to 'contents' during the copy_from_user() call. The proposed fix is:

int len;

+ if (i >= NVRAM_BYTES)
+ return -EINVAL;
len = (NVRAM_BYTES – i) < count ? (NVRAM_BYTES - i) : count; if (copy_from_user(contents, buf, len)) return -EFAULT; [/sourcecode] Which checks that 'i' cannot be larger than the size of 'contents' array. In case you wonder... Yes, this is a kernel stack smash but keep in mind that /dev/nvram isn't +rw in most cases. I checked this and it is present at least since 2.6.24 release of the Linux kernel. P.S.: spender, /dev/net/tun is +rw by default? :b

Written by xorl

July 18, 2009 at 03:20

Posted in bugs, linux

5 Responses

Subscribe to comments with RSS.

  1. It depends on what you installed by default, I believe. Of the people I asked, all of them had 666 /dev/net/tun. Its permissions may look differently until you actually try to use the device (then udev kicks in). On SuSE I hear it’s 666 by default ;)

    This bug makes me think of an improvement we can make to PAX_USERCOPY though:

    with copy_from_user being inlined, we can check to see if the destination exists within the current stack, and if so, perform a sizeof on that destination. If that size is > sizeof(long) then we assume it’s a static array or structure on the stack and limit the copy to that size. If this buffer had existed on the heap, PAX_USERCOPY would have already prevented the overflow.

    -Brad

    Bradley Spengler

    July 18, 2009 at 05:41

  2. Actually, this was more of a rhetoric question because of the nature of that nvram bug. However, thanks for your reply.

    I have seen some diffs that you have uploaded for copy_{to,from}_user that would make exploitation of such bugs extremely difficult if not impossible. BAD spender! :P

    Regarding the bug… ok, I’ll find another one “silently” fixed which is quite easy for the Linux kernel ;p

    xorl

    July 18, 2009 at 05:49

  3. NO PERSONAL CHECK

    thanasisk

    July 19, 2009 at 09:23

  4. just some suggests :
    – post about old linux kernel vulnerabilities too .
    – make another category for linux kernel
    – mark important lines of codes .
    – plz post more :)
    Thanks.

    0x29A

    July 19, 2009 at 14:34

  5. NO MORE FREE BUGS THANASISK!!! :P

    @0x29A:
    – Nice idea for the old Linux (or maybe others as well) kernel bugs (even though I hardly find time to post about the latest ones)
    – There is a category “Linux” which includes all of my Linux-related posts already.
    – Well, I think I’m discussing in some detail the important lines of code so no need to spend time on CSS or javascript stuff that I don’t like at all. :P
    – I’ll try
    Thank you too for the suggestions!

    xorl

    July 19, 2009 at 15:01


Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s