netdev - Re: [PATCH iproute2] iptunnel: detect protocol mismatch on tunnel change

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMw=ZnSzun2idW4LTozFPbAzU_vK=Lg1WN4tYTf2B2HsSDzB4g@mail.gmail.com>
Date:   Tue, 11 Apr 2023 11:09:35 +0100
From:   Luca Boccassi <bluca@...ian.org>
To:     Stephen Hemminger <stephen@...workplumber.org>
Cc:     netdev@...r.kernel.org
Subject: Re: [PATCH iproute2] iptunnel: detect protocol mismatch on tunnel change

On Tue, 11 Apr 2023 at 00:35, Stephen Hemminger
<stephen@...workplumber.org> wrote:
>
> If attempt is made to change an IPv6 tunnel by using IPv4
> parameters, a stack overflow would happen and garbage request
> would be passed to kernel.
>
> Example:
> ip tunnel add gre1 mode ip6gre local 2001:db8::1 remote 2001:db8::2 ttl 255
> ip tunnel change gre1 mode gre local 192.168.0.0 remote 192.168.0.1 ttl 255
>
> The second command should fail because it attempting set IPv4 addresses
> on a GRE tunnel that is IPv6.
>
> Do best effort detection of this mismatch by giving a bigger buffer to get
> tunnel request, and checking that the IP header is IPv4. It is still possible
> but unlikely that byte would match in IPv6 tunnel paramater, but good enough
> to catch the obvious cases.
>
> Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1032642
> Reported-by: Robin <imer@...r.cc>
> Signed-off-by: Stephen Hemminger <stephen@...workplumber.org>
> ---
>  ip/iptunnel.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/ip/iptunnel.c b/ip/iptunnel.c
> index 02c3670b469d..b6da145913d6 100644
> --- a/ip/iptunnel.c
> +++ b/ip/iptunnel.c
> @@ -17,6 +17,7 @@
>  #include <net/if_arp.h>
>  #include <linux/ip.h>
>  #include <linux/if_tunnel.h>
> +#include <linux/ip6_tunnel.h>
>
>  #include "rt_names.h"
>  #include "utils.h"
> @@ -172,11 +173,20 @@ static int parse_args(int argc, char **argv, int cmd, struct ip_tunnel_parm *p)
>                         if (get_ifname(p->name, *argv))
>                                 invarg("\"name\" not a valid ifname", *argv);
>                         if (cmd == SIOCCHGTUNNEL && count == 0) {
> -                               struct ip_tunnel_parm old_p = {};
> +                               union {
> +                                       struct ip_tunnel_parm ip_tnl;
> +                                       struct ip6_tnl_parm2 ip6_tnl;
> +                               } old_p = {};
>
>                                 if (tnl_get_ioctl(*argv, &old_p))
>                                         return -1;
> -                               *p = old_p;
> +
> +                               if (old_p.ip_tnl.iph.version != 4 ||
> +                                   old_p.ip_tnl.iph.ihl != 5)
> +                                       invarg("\"name\" is not an ip tunnel",
> +                                              *argv);
> +
> +                               *p = old_p.ip_tnl;
>                         }
>                 }
>                 count++;

Tested-by: Luca Boccassi <bluca@...ian.org>

Can confirm it doesn't crash anymore, thanks.