* [Qemu-devel] [PATCH] slirp: check sscanf result when emulating ident @ 2019-03-01 21:45 William Bowling 2019-03-02 2:21 ` Samuel Thibault 2019-03-02 17:42 ` Philippe Mathieu-Daudé 0 siblings, 2 replies; 5+ messages in thread From: William Bowling @ 2019-03-01 21:45 UTC (permalink / raw) To: qemu-devel; +Cc: secalert, William Bowling 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. Signed-off-by: William Bowling <will@wbowling.info> --- 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; -- 2.15.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] slirp: check sscanf result when emulating ident 2019-03-01 21:45 [Qemu-devel] [PATCH] slirp: check sscanf result when emulating ident William Bowling @ 2019-03-02 2:21 ` Samuel Thibault 2019-03-02 17:42 ` Philippe Mathieu-Daudé 1 sibling, 0 replies; 5+ messages in thread From: Samuel Thibault @ 2019-03-02 2:21 UTC (permalink / raw) To: William Bowling; +Cc: qemu-devel, secalert William Bowling, le ven. 01 mars 2019 21:45:56 +0000, a ecrit: > 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. > > Signed-off-by: William Bowling <will@wbowling.info> Applied to my tree, thanks! > --- > 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; > -- > 2.15.1 > > -- Samuel What's this script do? unzip ; touch ; finger ; mount ; gasp ; yes ; umount ; sleep Hint for the answer: not everything is computer-oriented. Sometimes you're in a sleeping bag, camping out. (Contributed by Frans van der Zande.) ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] slirp: check sscanf result when emulating ident 2019-03-01 21:45 [Qemu-devel] [PATCH] slirp: check sscanf result when emulating ident William Bowling 2019-03-02 2:21 ` Samuel Thibault @ 2019-03-02 17:42 ` Philippe Mathieu-Daudé 2019-03-02 17:49 ` Samuel Thibault 2019-03-02 23:28 ` William Bowling 1 sibling, 2 replies; 5+ messages in thread From: Philippe Mathieu-Daudé @ 2019-03-02 17:42 UTC (permalink / raw) To: William Bowling, qemu-devel; +Cc: secalert, qemu-stable 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 secalert@redhat.com, so that confirms my guess). > > Signed-off-by: William Bowling <will@wbowling.info> Reviewed-by: Philippe Mathieu-Daudé <philmd@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; > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] slirp: check sscanf result when emulating ident 2019-03-02 17:42 ` Philippe Mathieu-Daudé @ 2019-03-02 17:49 ` Samuel Thibault 2019-03-02 23:28 ` William Bowling 1 sibling, 0 replies; 5+ messages in thread From: Samuel Thibault @ 2019-03-02 17:49 UTC (permalink / raw) To: Philippe Mathieu-Daudé Cc: William Bowling, qemu-devel, qemu-stable, secalert Hello, Philippe Mathieu-Daudé, le sam. 02 mars 2019 18:42:42 +0100, a ecrit: > Samuel: since this diff is not obvious without looking at the context > (also due to the code re-indent), I dropped the code re-indent to make the change obvious. I still added the commit description, always better goes with saying it :) Thanks! Samuel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] slirp: check sscanf result when emulating ident 2019-03-02 17:42 ` Philippe Mathieu-Daudé 2019-03-02 17:49 ` Samuel Thibault @ 2019-03-02 23:28 ` William Bowling 1 sibling, 0 replies; 5+ messages in thread From: William Bowling @ 2019-03-02 23:28 UTC (permalink / raw) To: Philippe Mathieu-Daudé; +Cc: qemu-devel, secalert, qemu-stable 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 secalert@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é <philmd@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 secalert@redhat.com, so that > confirms my guess). > > > > > Signed-off-by: William Bowling <will@wbowling.info> > > Reviewed-by: Philippe Mathieu-Daudé <philmd@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 ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-03-02 23:28 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-03-01 21:45 [Qemu-devel] [PATCH] slirp: check sscanf result when emulating ident William Bowling 2019-03-02 2:21 ` Samuel Thibault 2019-03-02 17:42 ` Philippe Mathieu-Daudé 2019-03-02 17:49 ` Samuel Thibault 2019-03-02 23:28 ` William Bowling
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).