Re: XADV-2013004 Linux Kernel ipvs Kernel Stack Overflow

[Alan J. Wylie <shyyqvfpybfher () wylie me uk>]

x90c <geinblues () gmail com> writes:

> +--------------------------------------------------------+
> | XADV-2013004 Linux Kernel ipvs Kernel Stack Overflow   |
> +--------------------------------------------------------+
>
>  Vulnerable versions:
>  - linux kernel 2.6.32 <=
>
>  Not vulnerable versions:
>  - linux kernel 2.6.33 <=
                         ^^
ITYM >=

>  - linux kernel 3.x
>
>  Testbed: linux kernel 2.6.18
>  Type: Local
>  Impact: Local Privilege Escalation
>  Vendor: http://www.kernel.org
>  Author: x90c <geinblues *nospam* gmail dot com>
>  Site: x90c.org

...

> The do_ip_vs_set_ctl() in the ipvs is vulnerable function.
> It's vulnerable to the kernel stack overflow with no sanity check
> when copying the getsockopt socket option value from the userspace
> to the arg[] variable.

...

>     /* XXX no sanity check. (kernel stack overflow) */
>     if (copy_from_user(arg, user, len) != 0)

Fixed nearly four years ago.

------------------------------------------------------------------------------
commit 04bcef2a83f40c6db24222b27a52892cba39dffb
Author: Arjan van de Ven <arjan () linux intel com>
Date:   Mon Jan 4 16:37:12 2010 +0100

    ipvs: Add boundary check on ioctl arguments
    
    The ipvs code has a nifty system for doing the size of ioctl command
    copies; it defines an array with values into which it indexes the
    cmd
    to find the right length.
    
    Unfortunately, the ipvs code forgot to check if the cmd was in the
    range that the array provides, allowing for an index outside of the
    array, which then gives a "garbage" result into the length, which
    then gets used for copying into a stack buffer.
    
    Fix this by adding sanity checks on these as well as the copy size.
    
    [ horms () verge net au: adjusted limit to IP_VS_SO_GET_MAX ]
    Signed-off-by: Arjan van de Ven <arjan () linux intel com>
    Acked-by: Julian Anastasov <ja () ssi bg>
    Signed-off-by: Simon Horman <horms () verge net au>
    Signed-off-by: Patrick McHardy <kaber () trash net>

diff --git a/net/netfilter/ipvs/ip_vs_ctl.c
b/net/netfilter/ipvs/ip_vs_ctl.c
index 6bde12d..c37ac2d 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -2077,6 +2077,10 @@ do_ip_vs_set_ctl(struct sock *sk, int cmd, void
__user *user, unsigned int len)
        if (!capable(CAP_NET_ADMIN))
                return -EPERM;
 
+       if (cmd < IP_VS_BASE_CTL || cmd > IP_VS_SO_SET_MAX)
+               return -EINVAL;
+       if (len < 0 || len >  MAX_ARG_LEN)
+               return -EINVAL;
        if (len != set_arglen[SET_CMDID(cmd)]) {
                pr_err("set_ctl: len %u != %u\n",
                       len, set_arglen[SET_CMDID(cmd)]);

------------------------------------------------------------------------------

-- 
Alan J. Wylie                                          http://www.wylie.me.uk/

_______________________________________________
Full-Disclosure - We believe in it.
Charter: http://lists.grok.org.uk/full-disclosure-charter.html
Hosted and sponsored by Secunia - http://secunia.com/