netdev - Re: [PATCH] x25: Validate incoming call user data lengths

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1317738819.2440.9.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>
Date:	Tue, 04 Oct 2011 16:33:39 +0200
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Matthew Daley <mattjd@...il.com>
Cc:	netdev@...r.kernel.org, Andrew Hendry <andrew.hendry@...il.com>
Subject: Re: [PATCH] x25: Validate incoming call user data lengths

Le mardi 04 octobre 2011 à 14:00 +0000, Matthew Daley a écrit :
> X.25 call user data is being copied in its entirety from incoming messages
> without consideration to the size of the destination buffers, leading to
> possible buffer overflows. Validate incoming call user data lengths before
> these copies are performed.
> 
> It appears this issue was noticed some time ago, however nothing seemed to
> come of it: see http://www.spinics.net/lists/linux-x25/msg00043.html and
> commit 8db09f26f912f7c90c764806e804b558da520d4f.
> 
> Signed-off-by: Matthew Daley <mattjd@...il.com>
> Cc: Eric Dumazet <eric.dumazet@...il.com>
> Cc: Andrew Hendry <andrew.hendry@...il.com>
> Cc: stable <stable@...nel.org>
> ---
> Hopefully this is acceptable; I thought someone with greater knowledge of
> X.25 might step in :) It should take into account Andrew's suggestion to
> move af_x25.c's check to before the call acception.
> 
>  net/x25/af_x25.c |    6 ++++++
>  net/x25/x25_in.c |    3 +++
>  2 files changed, 9 insertions(+), 0 deletions(-)
> 
> diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
> index d306154..a4bd172 100644
> --- a/net/x25/af_x25.c
> +++ b/net/x25/af_x25.c
> @@ -959,6 +959,12 @@ int x25_rx_call_request(struct sk_buff *skb,
> struct x25_neigh *nb,
>  	skb_pull(skb,len);
> 
>  	/*
> +	 *	Ensure that the amount of call user data is valid.
> +	 */
> +	if (skb->len > X25_MAX_CUD_LEN)
> +		goto out_clear_request;
> +
> +	/*
>  	 *	Find a listener for the particular address/cud pair.
>  	 */
>  	sk = x25_find_listener(&source_addr,skb);
> diff --git a/net/x25/x25_in.c b/net/x25/x25_in.c
> index 0b073b5..63488fd 100644
> --- a/net/x25/x25_in.c
> +++ b/net/x25/x25_in.c
> @@ -127,6 +127,9 @@ static int x25_state1_machine(struct sock *sk,
> struct sk_buff *skb, int frametyp
>  		 *	Copy any Call User Data.
>  		 */
>  		if (skb->len > 0) {
> +			if (skb->len > X25_MAX_CUD_LEN)
> +				goto out_clear;
> +
>  			skb_copy_from_linear_data(skb,
>  						  x25->calluserdata.cuddata,
>  						  skb->len);
> --

It seems fine to me.

Acked-by: Eric Dumazet <eric.dumazet@...il.com>

Now, you could also fix x25 for undersized/frags frames :

Apparently, no check is done in this respect (missing pskb_may_pull()
calls)...



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html