Re: [Qemu-devel] [PATCH] slirp: check sscanf result when emulating ident

[William Bowling]

Hi Phil,

William: How did you notice that? Using a static analyzer?


It was while looking into a previous CVE in tcp_emu, just with a manual
code review.

We have a data leak, Cc'ing qemu-stable.
> (Adding the address I noticed you Cc'ed address@hidden, so that
> confirms my guess).


Yeah the report and patch went via the security list initially due to the
info leak.

Cheers,
Will

On Sun, Mar 3, 2019 at 4:42 AM Philippe Mathieu-Daudé <address@hidden>
wrote:

> Hi William, Samuel,
>
> On 3/1/19 10:45 PM, William Bowling wrote:
> > When emulating ident in tcp_emu, if the strchr checks passed but the
> > sscanf check failed, two uninitialized variables would be copied and
> > sent in the reply.
>
> William: How did you notice that? Using a static analyzer?
>
> Samuel: since this diff is not obvious without looking at the context
> (also due to the code re-indent), can you improve the commit
> description, such (or better):
>
> "Move this code inside the if(sscanf()) clause".
>
> We have a data leak, Cc'ing qemu-stable.
> (Adding the address I noticed you Cc'ed address@hidden, so that
> confirms my guess).
>
> >
> > Signed-off-by: William Bowling <address@hidden>
>
> Reviewed-by: Philippe Mathieu-Daudé <address@hidden>
>
> Thanks,
>
> Phil.
>
> > ---
> >  slirp/tcp_subr.c | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/slirp/tcp_subr.c b/slirp/tcp_subr.c
> > index 262a42d6c8..73a160ba16 100644
> > --- a/slirp/tcp_subr.c
> > +++ b/slirp/tcp_subr.c
> > @@ -664,12 +664,12 @@ tcp_emu(struct socket *so, struct mbuf *m)
> >                                                       break;
> >                                               }
> >                                       }
> > -                             }
> > -                                so_rcv->sb_cc =
> snprintf(so_rcv->sb_data,
> > -
>  so_rcv->sb_datalen,
> > -                                                         "%d,%d\r\n",
> n1, n2);
> > -                             so_rcv->sb_rptr = so_rcv->sb_data;
> > -                             so_rcv->sb_wptr = so_rcv->sb_data +
> so_rcv->sb_cc;
> > +                    so_rcv->sb_cc = snprintf(so_rcv->sb_data,
> > +                                             so_rcv->sb_datalen,
> > +                                             "%d,%d\r\n", n1, n2);
> > +                    so_rcv->sb_rptr = so_rcv->sb_data;
> > +                    so_rcv->sb_wptr = so_rcv->sb_data + so_rcv->sb_cc;
> > +                }
> >                       }
> >                       m_free(m);
> >                       return 0;
> >
>


-- 

GPG Key ID: 0x980F711A

GPG Key Fingerprint: AA38 2A0E 7D22 18A9 6086  0289 41DC E04B 980F 711A