diff mbox

slirp: udp: fix NULL pointer dereference because of uninitialized socket

Message ID 20140917114130.GT9321@dhcp-25-225.brq.redhat.com
State New
Headers show

Commit Message

Petr Matousek Sept. 17, 2014, 11:41 a.m. UTC
When guest sends udp packet with source port and source addr 0,
uninitialized socket is picked up when looking for matching and already
created udp sockets, and later passed to sosendto() where NULL pointer
dereference is hit during so->slirp->vnetwork_mask.s_addr access.

Fix this by checking that the socket is in initialized state.

This is CVE-2014-3640.

Signed-off-by: Petr Matousek <pmatouse@redhat.com>
Reported-by: Xavier Mehrenberger <xavier.mehrenberger@airbus.com>
Reported-by: Stephane Duverger <stephane.duverger@eads.net>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
---
 slirp/udp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Michael S. Tsirkin Sept. 17, 2014, 2:18 p.m. UTC | #1
On Wed, Sep 17, 2014 at 01:41:30PM +0200, Petr Matousek wrote:
> When guest sends udp packet with source port and source addr 0,
> uninitialized socket is picked up when looking for matching and already
> created udp sockets, and later passed to sosendto() where NULL pointer
> dereference is hit during so->slirp->vnetwork_mask.s_addr access.
> 
> Fix this by checking that the socket is in initialized state.
> 
> This is CVE-2014-3640.
> 
> Signed-off-by: Petr Matousek <pmatouse@redhat.com>
> Reported-by: Xavier Mehrenberger <xavier.mehrenberger@airbus.com>
> Reported-by: Stephane Duverger <stephane.duverger@eads.net>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

Security bug so

Cc: qemu-stable@nongnu.org

> ---
>  slirp/udp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/slirp/udp.c b/slirp/udp.c
> index 8cc6cb6..4affcc6 100644
> --- a/slirp/udp.c
> +++ b/slirp/udp.c
> @@ -152,7 +152,7 @@ udp_input(register struct mbuf *m, int iphlen)
>  	 * Locate pcb for datagram.
>  	 */
>  	so = slirp->udp_last_so;
> -	if (so->so_lport != uh->uh_sport ||
> +	if (!so->so_state || so->so_lport != uh->uh_sport ||
>  	    so->so_laddr.s_addr != ip->ip_src.s_addr) {
>  		struct socket *tmp;
>  
> -- 
> 1.9.3
Jan Kiszka Sept. 17, 2014, 4 p.m. UTC | #2
On 2014-09-17 13:41, Petr Matousek wrote:
> When guest sends udp packet with source port and source addr 0,
> uninitialized socket is picked up when looking for matching and already
> created udp sockets, and later passed to sosendto() where NULL pointer
> dereference is hit during so->slirp->vnetwork_mask.s_addr access.
> 
> Fix this by checking that the socket is in initialized state.
> 
> This is CVE-2014-3640.
> 
> Signed-off-by: Petr Matousek <pmatouse@redhat.com>
> Reported-by: Xavier Mehrenberger <xavier.mehrenberger@airbus.com>
> Reported-by: Stephane Duverger <stephane.duverger@eads.net>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  slirp/udp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/slirp/udp.c b/slirp/udp.c
> index 8cc6cb6..4affcc6 100644
> --- a/slirp/udp.c
> +++ b/slirp/udp.c
> @@ -152,7 +152,7 @@ udp_input(register struct mbuf *m, int iphlen)
>  	 * Locate pcb for datagram.
>  	 */
>  	so = slirp->udp_last_so;
> -	if (so->so_lport != uh->uh_sport ||
> +	if (!so->so_state || so->so_lport != uh->uh_sport ||

I think it would be clearer to test so against &slirp->udb. That's done
later on in this code as well to differentiate between valid sockets and
this stub.

Jan

>  	    so->so_laddr.s_addr != ip->ip_src.s_addr) {
>  		struct socket *tmp;
>  
>
diff mbox

Patch

diff --git a/slirp/udp.c b/slirp/udp.c
index 8cc6cb6..4affcc6 100644
--- a/slirp/udp.c
+++ b/slirp/udp.c
@@ -152,7 +152,7 @@  udp_input(register struct mbuf *m, int iphlen)
 	 * Locate pcb for datagram.
 	 */
 	so = slirp->udp_last_so;
-	if (so->so_lport != uh->uh_sport ||
+	if (!so->so_state || so->so_lport != uh->uh_sport ||
 	    so->so_laddr.s_addr != ip->ip_src.s_addr) {
 		struct socket *tmp;