Message-ID: <20211016165415.3f4e8842@hermes.local>
Date:   Sat, 16 Oct 2021 16:54:15 -0700
From:   Stephen Hemminger <stephen@...workplumber.org>
To:     netdev@...r.kernel.org
Subject: Fw: [Bug 214741] New: Information leakage from kernel to user space
 in /net/netrom/af_netrom.c



Begin forwarded message:

Date: Sat, 16 Oct 2021 21:03:45 +0000
From: bugzilla-daemon@...zilla.kernel.org
To: stephen@...workplumber.org
Subject: [Bug 214741] New: Information leakage from kernel to user space in /net/netrom/af_netrom.c


https://bugzilla.kernel.org/show_bug.cgi?id=214741

            Bug ID: 214741
           Summary: Information leakage from kernel to user space in
                    /net/netrom/af_netrom.c
           Product: Networking
           Version: 2.5
    Kernel Version: 5.15-rc5
          Hardware: All
                OS: Linux
              Tree: Mainline
            Status: NEW
          Severity: high
          Priority: P1
         Component: Other
          Assignee: stephen@...workplumber.org
          Reporter: bao00065@....edu
        Regression: No

Hi Maintainers,
I recently reviewed the uninitialized value use bug in Linux kernel:

https://syzkaller.appspot.com/bug?id=739ce0bc6e4097668cbf94c862f3b643b364d589. 
and patch:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b42b3a2744b3e8f427de79896720c72823af91ad

The vulnerable function is:
int __sys_getsockname(int fd, struct sockaddr __user *usockaddr,
                      int __user *usockaddr_len)
{
        struct socket *sock;
        struct sockaddr_storage address; // allocation
        int err, fput_needed;

        sock = sockfd_lookup_light(fd, &err, &fput_needed);
        if (!sock)
                goto out;

        err = security_socket_getsockname(sock);
        if (err)
                goto out_put;

        err = sock->ops->getname(sock, (struct sockaddr *)&address, 0); //
initialization
        if (err < 0)
                goto out_put;
        /* "err" is actually length in this case */
        err = move_addr_to_user(&address, err, usockaddr, usockaddr_len); //use

out_put:
        fput_light(sock->file, fput_needed);
out:
        return err;
}

static int move_addr_to_user(struct sockaddr_storage *kaddr, int klen,
                             void __user *uaddr, int __user *ulen)
{
        int err;
        int len;

        BUG_ON(klen > sizeof(struct sockaddr_storage));
        err = get_user(len, ulen);
        if (err)
                return err;
        if (len > klen)
                len = klen;
        if (len < 0)
                return -EINVAL;
        if (len) {
                if (audit_sockaddr(klen, kaddr))
                        return -ENOMEM;
                if (copy_to_user(uaddr, kaddr, len)) // use 
                        return -EFAULT;
        }
        /*
         *      "fromlen shall refer to the value before truncation.."
         *                      1003.1g
         */
        return __put_user(klen, ulen);
}


the variable address is allocated in __sys_getsockname, and then is initialized
in sock->ops->getname(sock, (struct sockaddr *)& address, 0); After that,
address is passed to move_addr_to_user() and finally it passed to
copy_to_user(), leading to uninitialized value use. 

Main reason for this bug: initialization in sock->ops->getname(sock, (struct
sockaddr *)&address, 0) is partially initialized.  It only initializes the
fields in the struct but not the holes between the fields. As a result, since
uaddr will be passed to copy_to_user(), and the holes inside the uaddr struct 
contain uninitialized data inherited from the kernel stack, it may cause
information leakage from kernel space to user space

Here is the initialization function:

static int isotp_getname(struct socket *sock, struct sockaddr *uaddr, int peer)
{
        struct sockaddr_can *addr = (struct sockaddr_can *)uaddr;
        struct sock *sk = sock->sk;
        struct isotp_sock *so = isotp_sk(sk);

        if (peer)
                return -EOPNOTSUPP;

        +memset(addr, 0, ISOTP_MIN_NAMELEN);//patch
        addr->can_family = AF_CAN;
        addr->can_ifindex = so->ifindex;
        addr->can_addr.tp.rx_id = so->rxid;
        addr->can_addr.tp.tx_id = so->txid;

        return ISOTP_MIN_NAMELEN;
}

The patch: memset() initializes the whole struct sockaddr_can, including the
holes within the struct sockaddr_can.

Sockaddr_can is declared here:
struct sockaddr_can {
        __kernel_sa_family_t can_family;
        int         can_ifindex;
        union {
                /* transport protocol class address information (e.g. ISOTP) */
                struct { canid_t rx_id, tx_id; } tp;

                /* J1939 address information */
                struct {
                        /* 8 byte name when using dynamic addressing */
                        __u64 name;

                        /* pgn:
                         * 8 bit: PS in PDU2 case, else 0
                         * 8 bit: PF
                         * 1 bit: DP
                         * 1 bit: reserved
                         */
                        __u32 pgn;

                        /* 1 byte address */
                        __u8 addr;
                } j1939;

                /* reserved for future CAN protocols address information */
        } can_addr;
};
There are a few holes inside the struct, but it doesn’t explicitly set to 0 in 
isotp_getname()

At the same time, I realized  sock->ops->getname(sock, (struct sockaddr
*)&address, 0) is an indirect call. Thus it may go to different functions at
the run time. If one of these functions doesn't initialize the holes within the
struct, it may cause an information leak from kernel space to userspace. 


My tools find similar cloned bugs
The same bug happens in /net/netrom/af_netrom.c


static int nr_getname(struct socket *sock, struct sockaddr *uaddr,
        int peer)
{
        struct full_sockaddr_ax25 *sax = (struct full_sockaddr_ax25 *)uaddr;
        struct sock *sk = sock->sk;
        struct nr_sock *nr = nr_sk(sk);
        int uaddr_len;

        memset(&sax->fsa_ax25, 0, sizeof(struct sockaddr_ax25));

        lock_sock(sk);
        if (peer != 0) {
                if (sk->sk_state != TCP_ESTABLISHED) {
                        release_sock(sk);
                        return -ENOTCONN;
                }
                sax->fsa_ax25.sax25_family = AF_NETROM;
                sax->fsa_ax25.sax25_ndigis = 1;
                sax->fsa_ax25.sax25_call   = nr->user_addr;
                memset(sax->fsa_digipeater, 0, sizeof(sax->fsa_digipeater));
                sax->fsa_digipeater[0]     = nr->dest_addr;
                uaddr_len = sizeof(struct full_sockaddr_ax25);
        } else {
                sax->fsa_ax25.sax25_family = AF_NETROM;
                sax->fsa_ax25.sax25_ndigis = 0;
                sax->fsa_ax25.sax25_call   = nr->source_addr;
                uaddr_len = sizeof(struct sockaddr_ax25);
        }
        release_sock(sk);

        return uaddr_len;
}

full_sockaddr_ax25 is declared here:

struct full_sockaddr_ax25 {
        struct sockaddr_ax25 fsa_ax25;
        ax25_address    fsa_digipeater[AX25_MAX_DIGIS];
};

and sockaddr_ax25 is declared here:
struct sockaddr_ax25 {
        __kernel_sa_family_t sax25_family;
        ax25_address    sax25_call;
        int             sax25_ndigis;
        /* Digipeater ax25_address sets follow */
};

ax25_address is declared here:
typedef struct {
        char            ax25_call[7];   /* 6 call + SSID (shifted ascii!) */
} ax25_address;

We can see there are a few holes inside struct sockaddr_ax25. Thus, we have to
explicitly set the hole to zero. Otherwise, the address will be passed to
copy_to_user and cause information leakage.

Suggested patch:
memset(maddr, 0, sizeof(full_sockaddr_ax25));


Thank you for the review. I appreciate your time. 

Andrew Bao

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are the assignee for the bug.

Powered by blists - more mailing lists