netdev - Re: A kernel-infoleak bug in pppoe_getname() in drivers/net/ppp/pppoe.c

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: <20211103095308.7ff68a7f@kicinski-fedora-PC1C0HJN>
Date:   Wed, 3 Nov 2021 09:53:08 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     butt3rflyh4ck <butterflyhuangxx@...il.com>
Cc:     mostrows@...thlink.net, "David S. Miller" <davem@...emloft.net>,
        Networking <netdev@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Bhaskar Chowdhury <unixbhaskar@...il.com>,
        Guillaume Nault <gnault@...hat.com>
Subject: Re: A kernel-infoleak bug in pppoe_getname() in
 drivers/net/ppp/pppoe.c

On Thu, 4 Nov 2021 00:14:31 +0800 butt3rflyh4ck wrote:
> Hi, I report a kernel-infoleak bug in pppoe_getname()) in
> drivers/net/ppp/pppoe.c.
> And we can call getname ioctl to invoke pppoe_getname().
> 
> ###anaylze
> ```
> static int pppoe_getname(struct socket *sock, struct sockaddr *uaddr,
>   int peer)
> {
> int len = sizeof(struct sockaddr_pppox);
> struct sockaddr_pppox sp;    ///--->  define a 'sp' in stack but does
> not clear it
> 
> sp.sa_family = AF_PPPOX;   ///---> sp.sa_family is a short type, just

But the structure is marked as __packed.

> 2 byte sizes.
> sp.sa_protocol = PX_PROTO_OE;
> memcpy(&sp.sa_addr.pppoe, &pppox_sk(sock->sk)->pppoe_pa,
>        sizeof(struct pppoe_addr));
> 
> memcpy(uaddr, &sp, len);
> 
> return len;
> }
> ```
> There is an anonymous 2-byte hole after sa_family, make sure to clear it.
> 
> ###fix
> use memset() to clear the struct sockaddr_pppox sp.
> ```
> diff --git a/drivers/net/ppp/pppoe.c b/drivers/net/ppp/pppoe.c
> index 3619520340b7..fec328ad7202 100644
> --- a/drivers/net/ppp/pppoe.c
> +++ b/drivers/net/ppp/pppoe.c
> @@ -723,6 +723,11 @@ static int pppoe_getname(struct socket *sock,
> struct sockaddr *uaddr,
>         int len = sizeof(struct sockaddr_pppox);
>         struct sockaddr_pppox sp;
> 
> +       /* There is an anonymous 2-byte hole after sa_family,
> +        * make sure to clear it.
> +        */
> +       memset(&sp, 0, len);
> +
>         sp.sa_family    = AF_PPPOX;
>         sp.sa_protocol  = PX_PROTO_OE;
>         memcpy(&sp.sa_addr.pppoe, &pppox_sk(sock->sk)->pppoe_pa,
> ```
> The attachment is a patch.