netdev - Re: [PATCH net] pppoe: check sockaddr length in pppoe

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]

Date:   Fri, 27 Apr 2018 08:23:16 -0400
From:   Kevin Easton <kevin@...rana.org>
To:     Guillaume Nault <g.nault@...halink.fr>
Cc:     netdev@...r.kernel.org, Michal Ostrowski <mostrows@...thlink.net>
Subject: Re: [PATCH net] pppoe: check sockaddr length in pppoe_connect()

On Mon, Apr 23, 2018 at 04:38:27PM +0200, Guillaume Nault wrote:
> We must validate sockaddr_len, otherwise userspace can pass fewer data
> than we expect and we end up accessing invalid data.
> 
> Fixes: 224cf5ad14c0 ("ppp: Move the PPP drivers")
> Reported-by: syzbot+4f03bdf92fdf9ef5ddab@...kaller.appspotmail.com
> Signed-off-by: Guillaume Nault <g.nault@...halink.fr>
> ---
>  drivers/net/ppp/pppoe.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/ppp/pppoe.c b/drivers/net/ppp/pppoe.c
> index 1483bc7b01e1..7df07337d69c 100644
> --- a/drivers/net/ppp/pppoe.c
> +++ b/drivers/net/ppp/pppoe.c
> @@ -620,6 +620,10 @@ static int pppoe_connect(struct socket *sock, struct sockaddr *uservaddr,
>  	lock_sock(sk);
>  
>  	error = -EINVAL;
> +
> +	if (sockaddr_len != sizeof(struct sockaddr_pppox))
> +		goto end;
> +
>  	if (sp->sa_protocol != PX_PROTO_OE)
>  		goto end;

There's another bug here - pppoe_connect() should also be validating
sp->sa_family.  My suggested patch was going to be:

diff --git a/drivers/net/ppp/pppoe.c b/drivers/net/ppp/pppoe.c
index 1483bc7..90eb3fd 100644
--- a/drivers/net/ppp/pppoe.c
+++ b/drivers/net/ppp/pppoe.c
@@ -620,6 +620,14 @@ static int pppoe_connect(struct socket *sock, struct sockaddr *uservaddr,
        lock_sock(sk);
 
        error = -EINVAL;
+       if (sockaddr_len < sizeof(struct sockaddr_pppox))
+               goto end;
+
+       error = -EAFNOSUPPORT;
+       if (sp->sa_family != AF_PPPOX)
+               goto end;
+
+       error = -EINVAL;
        if (sp->sa_protocol != PX_PROTO_OE)
                goto end;
 
Should I rework this on top of net.git HEAD?

(The same applies to pppol2tp_connect()).

    - Kevin