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 secal...@redhat.com, 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é <phi...@redhat.com> 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 secal...@redhat.com, so that > confirms my guess). > > > > > Signed-off-by: William Bowling <w...@wbowling.info> > > Reviewed-by: Philippe Mathieu-Daudé <phi...@redhat.com> > > 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
