On Fri, Aug 23, 2013 at 12:47:10PM -0700, Loganaden Velvindron wrote:
> Hi,
> 
> >From NetBSD:
> 
> http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/netinet6/udp6_output.c?rev=1.41&content-type=text/x-cvsweb-markup&only_with_tag=MAIN
> "
> Under some circumstances, udp6_output() would call ip6_clearpktopts()
> with an uninitialized struct ip6_pktopts on the stack, opt.
> ip6_clearpktopts(&opt, ...) could dereference dangling pointers,
> leading to memory corruption or a crash.  Now, udp6_output() calls
> ip6_clearpktopts(&opt, ...) only if opt was initialized. Thanks to
> Clement LECIGNE for reporting this bug."
> 
> I checked openbsd source code and it seems that the issue is present
> as well.

No, this doesn't apply to OpenBSD.

In OpenBSD, udp6_output() starts off with a call to ip6_setpktopts()
if control is not NULL. This means that opt will always be initalised
before being cleaned up.

NetBSD does not initialise opt at the start of udp6_output().
Rather, they initalise it after an if (addr6) { ... } block of
code which in case of error can jump to the code that tries to
free opt (via 'goto release;'), without opt having been initalised.
That's what the extra if (optp == &opt) is supposed to prevent.

The problem was introduced in NetBSD in this revision:
http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/netinet6/udp6_output.c.diff?r1=1.22&r2=1.23&only_with_tag=MAIN

> Tentative diff:
> 
> Index: udp6_output.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet6/udp6_output.c,v
> retrieving revision 1.19
> diff -u -p -r1.19 udp6_output.c
> --- udp6_output.c     28 Mar 2013 16:45:16 -0000      1.19
> +++ udp6_output.c     23 Aug 2013 19:30:36 -0000
> @@ -119,7 +119,8 @@ udp6_output(struct in6pcb *in6p, struct 
>       struct  in6_addr *laddr, *faddr;
>       u_short fport;
>       int error = 0;
> -     struct ip6_pktopts *optp, opt;
> +     struct ip6_pktopts *optp = NULL; 
> +     struct ip6_pktopts opt;
>       int priv;
>       int af, hlen;
>       int flags;
> @@ -284,7 +285,8 @@ release:
>  
>  releaseopt:
>       if (control) {
> -             ip6_clearpktopts(&opt, -1);
> +             if (optp == &opt)
> +                     ip6_clearpktopts(&opt, -1);
>               m_freem(control);
>       }
>       return (error);

Reply via email to