netdev - Re: [PATCH iproute2] iproute2: fix use-after-free

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 for Android: free password hash cracker in your pocket

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

Date:   Wed, 12 Sep 2018 23:07:20 -0700
From:   Mahesh Bandewar (महेश बंडेवार) 
        <maheshb@...gle.com>
To:     Stephen Hemminger <stephen@...workplumber.org>
Cc:     Mahesh Bandewar <mahesh@...dewar.net>,
        netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH iproute2] iproute2: fix use-after-free

On Wed, Sep 12, 2018 at 5:33 PM, Stephen Hemminger
<stephen@...workplumber.org> wrote:
>
> On Wed, 12 Sep 2018 16:29:28 -0700
> Mahesh Bandewar <mahesh@...dewar.net> wrote:
>
> > From: Mahesh Bandewar <maheshb@...gle.com>
> >
> > A local program using iproute2 lib pointed out the issue and looking
> > at the code it is pretty obvious -
> >
> >     a = (struct nlmsghdr *)b;
> >     ...
> >     free(b);
> >     if (a->nlmsg_seq == seq)
> >     ...
> >
> > Fixes: 86bf43c7c2fd ("lib/libnetlink: update rtnl_talk to support malloc buff at run time")
> > Signed-off-by: Mahesh Bandewar <maheshb@...gle.com>
>
> Yes, this is a real problem.
>
> Maybe a minimal patch like this would be enough:
actually that will leave the memory leak at the 'goto next' line (just
few lines below) since that jump will overwrite the buf.

> diff --git a/lib/libnetlink.c b/lib/libnetlink.c
> index 928de1dd16d8..ab2d8452e4a1 100644
> --- a/lib/libnetlink.c
> +++ b/lib/libnetlink.c
> @@ -661,6 +661,8 @@ next:
>                                 if (l < sizeof(struct nlmsgerr)) {
>                                         fprintf(stderr, "ERROR truncated\n");
>                                 } else if (!err->error) {
> +                                       __u32 err_seq = h->nlmsg_seq;
> +
>                                         /* check messages from kernel */
>                                         nl_dump_ext_ack(h, errfn);
>
> @@ -668,7 +670,8 @@ next:
>                                                 *answer = (struct nlmsghdr *)buf;
>                                         else
>                                                 free(buf);
> -                                       if (h->nlmsg_seq == seq)
> +
> +                                       if (err_seq == seq)
>                                                 return 0;
>                                         else if (i < iovlen)
>                                                 goto next;
>