Hi All,
at the suggestion of Chris ([email protected]) I wrote a simple
checker to warn when the length parameter to copy_*_user was (1) an
integer and (2) not checked < 0.
As an example, the ipv6 routine rawv6_geticmpfilter gets an integer 'len'
from user space, checks that it is smaller than a struct size and then
uses length as an argument to copy_to_user:
if (get_user(len, optlen))
return -EFAULT;
if (len > sizeof(struct icmp6_filter))
len = sizeof(struct icmp6_filter);
if (put_user(len, optlen))
return -EFAULT;
if (copy_to_user(optval, &sk->tp_pinfo.tp_raw.filter, len))
return -EFAULT;
Is this a real bug? Or is the checked rule only applicable to
__copy_*_user routines rather than copy_*_user routines? (If its a real
bug, theres about 8 others that we found).
Thanks,
Dawson
On Tue, Apr 17, 2001 at 09:39:15PM -0700, Dawson Engler wrote:
> Hi All,
>
> at the suggestion of Chris ([email protected]) I wrote a simple
> checker to warn when the length parameter to copy_*_user was (1) an
> integer and (2) not checked < 0.
>
> As an example, the ipv6 routine rawv6_geticmpfilter gets an integer 'len'
> from user space, checks that it is smaller than a struct size and then
> uses length as an argument to copy_to_user:
>
> if (get_user(len, optlen))
> return -EFAULT;
> if (len > sizeof(struct icmp6_filter))
> len = sizeof(struct icmp6_filter);
> if (put_user(len, optlen))
> return -EFAULT;
> if (copy_to_user(optval, &sk->tp_pinfo.tp_raw.filter, len))
> return -EFAULT;
>
> Is this a real bug? Or is the checked rule only applicable to
> __copy_*_user routines rather than copy_*_user routines? (If its a real
> bug, theres about 8 others that we found).
The len parameter is an unsigned value, so this code is ok as
long as access_ok() correctly checks that the range to copy
doesn't stray outside of the userspace range, including the
possible wraparound for a very large len. access_ok() on i386
checks for the wraparound. m68k doesn't use it. PowerPC
is correct, but only because TASK_SIZE is 0x80000000. If it
is ever changed, there could be a problem. I didn't check
other architectures, because I don't understand the asm.
dave...
On Wed, 18 Apr 2001, David Schleef wrote:
> On Tue, Apr 17, 2001 at 09:39:15PM -0700, Dawson Engler wrote:
> > Hi All,
> >
> > at the suggestion of Chris ([email protected]) I wrote a simple
> > checker to warn when the length parameter to copy_*_user was (1) an
> > integer and (2) not checked < 0.
> >
> > As an example, the ipv6 routine rawv6_geticmpfilter gets an integer 'len'
> > from user space, checks that it is smaller than a struct size and then
> > uses length as an argument to copy_to_user:
> >
> > if (get_user(len, optlen))
> > return -EFAULT;
> > if (len > sizeof(struct icmp6_filter))
> > len = sizeof(struct icmp6_filter);
> > if (put_user(len, optlen))
> > return -EFAULT;
> > if (copy_to_user(optval, &sk->tp_pinfo.tp_raw.filter, len))
> > return -EFAULT;
> >
> > Is this a real bug? Or is the checked rule only applicable to
> > __copy_*_user routines rather than copy_*_user routines? (If its a real
> > bug, theres about 8 others that we found).
>
> The len parameter is an unsigned value, so this code is ok as
> long as access_ok() correctly checks that the range to copy
> doesn't stray outside of the userspace range, including the
> possible wraparound for a very large len. access_ok() on i386
> checks for the wraparound. m68k doesn't use it. PowerPC
> is correct, but only because TASK_SIZE is 0x80000000. If it
> is ever changed, there could be a problem. I didn't check
> other architectures, because I don't understand the asm.
Incorrect - if the "len" variable is a signed integer, this is a nasty
bug.
To justify this, consider if len were set to minus 2 billion. This will
pass the sanity check, and pass the value straight on to copy_to_user. The
copy_to_user parameter is unsigned, so this value because approximately
+2Gb.
Now, providing the malicious user passes a low user space pointer (e.g.
just above 0), the kernel's virtual address space wrap check will not
trigger because ~0 + ~2Gb does not exceed 4G. And the result is the user
being able to read kernel memory.
Cheers
Chris
On Tue, 17 Apr 2001, Dawson Engler wrote:
> Hi All,
>
> at the suggestion of Chris ([email protected]) I wrote a simple
> checker to warn when the length parameter to copy_*_user was (1) an
> integer and (2) not checked < 0.
>
> As an example, the ipv6 routine rawv6_geticmpfilter gets an integer 'len'
> from user space, checks that it is smaller than a struct size and then
> uses length as an argument to copy_to_user:
>
> if (get_user(len, optlen))
> return -EFAULT;
> if (len > sizeof(struct icmp6_filter))
> len = sizeof(struct icmp6_filter);
> if (put_user(len, optlen))
> return -EFAULT;
> if (copy_to_user(optval, &sk->tp_pinfo.tp_raw.filter, len))
> return -EFAULT;
>
> Is this a real bug? Or is the checked rule only applicable to
> __copy_*_user routines rather than copy_*_user routines? (If its a real
> bug, theres about 8 others that we found).
>
> Thanks,
> Dawson
Length is type size_t (unsigned). It can't be less than zero.
copy_to_user() should fault with invalid lengths. That's one
of the things it's supposed to do.
Cheers,
Dick Johnson
Penguin : Linux version 2.4.1 on an i686 machine (799.53 BogoMips).
"Memory is like gasoline. You use it up when you are running. Of
course you get it all back when you reboot..."; Actual explanation
obtained from the Micro$oft help desk.
On Wed, Apr 18, 2001 at 12:14:56PM +0100, Chris Evans wrote:
> To justify this, consider if len were set to minus 2 billion. This will
> pass the sanity check, and pass the value straight on to copy_to_user. The
> copy_to_user parameter is unsigned, so this value because approximately
> +2Gb.
For ARM, this isn't a problem (we do 33-bit arithmetic in access_ok
specifically to catch this type of thing). x86 does the same thing (or
did when I wrote the code for ARM.
> Now, providing the malicious user passes a low user space pointer (e.g.
> just above 0), the kernel's virtual address space wrap check will not
> trigger because ~0 + ~2Gb does not exceed 4G. And the result is the user
> being able to read kernel memory.
But ~0 + ~2GB = ~2GB. Last time I checked, ~2GB is less than 3GB, and 3GB
is the start of kernel memory on x86. Therefore, I don't see that the
user will be able to read kernel memory.
--
Russell King ([email protected]) The developer of ARM Linux
http://www.arm.linux.org.uk/personal/aboutme.html
On Wed, 18 Apr 2001, Russell King wrote:
> > Now, providing the malicious user passes a low user space pointer (e.g.
> > just above 0), the kernel's virtual address space wrap check will not
> > trigger because ~0 + ~2Gb does not exceed 4G. And the result is the user
> > being able to read kernel memory.
>
> But ~0 + ~2GB = ~2GB. Last time I checked, ~2GB is less than 3GB, and 3GB
> is the start of kernel memory on x86. Therefore, I don't see that the
> user will be able to read kernel memory.
The problem is that (up to) a 2Gb copy is attempted into userspace. The
source is a kernel object which is not 2Gb large! So, we read off the end
of some kernel object, and there is often something very interesting after
it ;-)
For a good real-world example, please see my Bugtraq post regarding
sysctl():
http://www.securityfocus.com/archive/1/161764
Cheers
Chris
Chris Evans <[email protected]> writes:
|> To justify this, consider if len were set to minus 2 billion. This will
|> pass the sanity check, and pass the value straight on to copy_to_user. The
|> copy_to_user parameter is unsigned, so this value because approximately
|> +2Gb.
|>
|> Now, providing the malicious user passes a low user space pointer (e.g.
|> just above 0), the kernel's virtual address space wrap check will not
|> trigger because ~0 + ~2Gb does not exceed 4G. And the result is the user
|> being able to read kernel memory.
On m68k this is not a problem, since kernel and user address space are
strictly distinct, even in the kernel. The luser will get an EFAULT
eventually.
Andreas.
--
Andreas Schwab "And now for something
SuSE Labs completely different."
[email protected]
SuSE GmbH, Schanz?ckerstr. 10, D-90443 N?rnberg
Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5