Re: [Full-Disclosure] Linux Kernel sctp_setsockopt() Integer Overflow

[Tom Rini]

On Tue, May 11, 2004 at 07:58:56PM +0100, Shaun Colley wrote:

> ~*~*~*~*~*~*~*~*~*~*~*~*~*~*~*~*~*~*~*~*~*~*~*~*~* ~*~*~*
>
> Product: Linux Kernel
> Versions: <= 2.4.25

Strictly speaking, 2.4.23-pre5 until 2.4.26.

> Bug: Integer overflow
> Impact: Attackers may be able to execute
> arbitrary code with kernel-level
> privileges.
> Risk: High
> Date: May 11, 2004
> Author: Shaun Colley
> Email: shaunige yahoo co uk
> WWW: http://www.nettwerked.co.uk
[snip]

--
Tom Rini
http://gate.crashing.org/~trini/

[Michael Tokarev]

Shaun Colley wrote:
[]
> Below is the vulnerable call:
>
> ---
> if (NULL == (tmp = kmalloc(optlen + 1, GFP_KERNEL))) {
> retval = -ENOMEM;
> goto out_unlock;
> }
> ---
>
> Because kmalloc() takes the 'count' variable as an
> unsigned number, negative numbers are interpreted as
> large unsigned numbers. However, if -1 is passed as
> 'optlen' (represented as 0xffffffff (hex) in unsigned
> variables, which is the largest value an unsigned
.....
[]
> And thus, due to the integer overflow, 0 is passed to
> kmalloc(), causing too little memory to be allocated
> to hold 'optval'.

But kmalloc(0) will return NULL, and the whole setsockopt
will finish with errno set to ENOMEM.

From 2.4 mm/slab.c:

void * kmalloc (size_t size, int flags)
{
cache_sizes_t *csizep = cache_sizes;

for (; csizep->cs_size; csizep++) {
if (size > csizep->cs_size)
continue;
return __kmem_cache_alloc(flags & GFP_DMA ?
csizep->cs_dmacachep : csizep->cs_cachep, flags);
}
return NULL;
}

So, where's the bug?

/mjt

[Michael Tokarev]

Bringing up an old topic (discussed 15-May), because
it seems noone replied to my post, which contains er..
wrong claims.

Michael Tokarev wrote:
> Shaun Colley wrote:
>>
>> ---
>> if (NULL == (tmp = kmalloc(optlen + 1, GFP_KERNEL))) {
>> retval = -ENOMEM;
>> goto out_unlock;
>> }
>> ---
>>
>> Because kmalloc() takes the 'count' variable as an
>> unsigned number, negative numbers are interpreted as
>> large unsigned numbers. However, if -1 is passed as
>> 'optlen' (represented as 0xffffffff (hex) in unsigned
>> variables, which is the largest value an unsigned
>
> .....
> []
>> And thus, due to the integer overflow, 0 is passed to
>> kmalloc(), causing too little memory to be allocated
>> to hold 'optval'.

> But kmalloc(0) will return NULL, and the whole setsockopt
> will finish with errno set to ENOMEM.
>
> From 2.4 mm/slab.c:
>
> void * kmalloc (size_t size, int flags)
> {
> cache_sizes_t *csizep = cache_sizes;
>
> for (; csizep->cs_size; csizep++) {
> if (size > csizep->cs_size)
> continue;
> return __kmem_cache_alloc(flags & GFP_DMA ?
> csizep->cs_dmacachep : csizep->cs_cachep, flags);
> }
> return NULL;
> }
>
> So, where's the bug?

I was wrong reading the above code, simple as that.
Sure, kmalloc(0) will NOT return NULL as I claimed.

if (size > csizep->cs_size)
continue;
Here, when size == 0 (and csizep->cs_size is always > 0),
the condition is always false, so the next instruction
will be executed, which is:

return __kmem_cache_alloc(flags & GFP_DMA ?
csizep->cs_dmacachep : csizep->cs_cachep, flags);

which will allocate either 32 or 64 bytes of memory (depending
on the arch) and return it to the caller.

So there IS a bug, exactly as described in the original advisory.

I wonder why noone replied... ;)

/mjt

[Jirka Kosina]

On Thu, 27 May 2004, Michael Tokarev wrote:

> I was wrong reading the above code, simple as that.
> Sure, kmalloc(0) will NOT return NULL as I claimed.
> if (size > csizep->cs_size)
> continue;
> Here, when size == 0 (and csizep->cs_size is always > 0),
> the condition is always false, so the next instruction
> will be executed, which is:
> return __kmem_cache_alloc(flags & GFP_DMA ?
> csizep->cs_dmacachep : csizep->cs_cachep, flags);
> which will allocate either 32 or 64 bytes of memory (depending
> on the arch) and return it to the caller.
> So there IS a bug, exactly as described in the original advisory.
> I wonder why noone replied... ;)

Because this all is debate about nothing, as the original advisory was
fake, because you simply can't pass negative optlen to setsockopt()
syscall, so there is nothing to be exploited.

asmlinkage long sys_setsockopt(int fd, int level, int optname, char __user
*optval,
{
int err;
struct socket *sock;

if (optlen < 0)
return -EINVAL;
....

--
JiKos.

[Shaun Colley]

> Because this all is debate about nothing, as the
> original advisory was
> fake, because you simply can't pass negative optlen
> to setsockopt()
> syscall, so there is nothing to be exploited.

No, the advisory was not fake. At the time, I didn't
realise that -1 or any negative will not get past
sys_setsockopt(). Without the sanity check in
setsockopt, there would be a bad security issue,
though. It's still worth upgrading, anyway. The bug
exists, just not a very big possibility of exploiting.



Thank you for your time.
Shaun.





__________________________________________________ __________
Yahoo! Messenger - Communicate instantly..."Ping"
your friends today! Download Messenger Now
http://uk.messenger.yahoo.com/download/index.html