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
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.)
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; >
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
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
© 2016 - 2025 Red Hat, Inc.