full-disclosure - Re: [FD] popo2, kernel/tun driver bufferoverflow.

Open Source and information security mailing list archives

Message-ID: <CAP145phfQ=keU=GUb7rjnf0rMja8OCyTjOaxeJYdNv+DcFJpQg@mail.gmail.com>
Date: Tue, 15 Jun 2021 10:32:47 +0200
From: Robert Święcki <robert@...ecki.net>
To: KJ Jung <x90cx90c1@...il.com>
Cc: fulldisclosure@...lists.org
Subject: Re: [FD] popo2, kernel/tun driver bufferoverflow.

Hi,

wt., 15 cze 2021 o 09:56 KJ Jung <x90cx90c1@...il.com> napisał(a):
>
> Linux kernel 5.4 version.
> latest.
>
> __tun_chr_ioctl function of ~/drivers/net/tun.c has a stack buffer
> overflow vulnerability. it get's arg, ifreq_len, and copy the arg(argp)
> to ifr(ifreq struct) and this steps are no bounds-checking.

While I agree that it might be not the best of programming patterns to
accept length of a local stack buffer from the parent function (this
can easily be misused over time), there's probably no bug here, as all
callers of __tun_chr_ioctl() use sizeof(struct ifreq) or sizeof(struct
compat_ifreq)) which is presumably shorter than the former one, no?
Or, maybe I'm missing something?

> if cmd == TUNSETIFF or TUNSETQUEUE or and so on condition
> then it's enter copy_from_user function area.
>
> --
> 3352static long tun_chr_ioctl(struct file *file,
> 3353                          unsigned int cmd, unsigned long arg)
> 3354{
> 3355        return __tun_chr_ioctl(file, cmd, arg, sizeof (struct ifreq));
> 3356}
>
> 3475static const struct file_operations tun_fops = {
> 3476        .owner  = THIS_MODULE,
> 3477        .llseek = no_llseek,
> 3478        .read_iter  = tun_chr_read_iter,
> 3479        .write_iter = tun_chr_write_iter,
> 3480        .poll   = tun_chr_poll,
> 3481        .unlocked_ioctl = tun_chr_ioctl,
>
> --
> https://lxr.missinglinkelectronics.com/linux/drivers/net/tun.c
>
> 3025static long __tun_chr_ioctl(struct file *file, unsigned int cmd,
> 3026                            unsigned long arg, int ifreq_len)
> 3027{
> 3028        struct tun_file *tfile = file->private_data;
> 3029        struct net *net = sock_net(&tfile->sk);
> 3030        struct tun_struct *tun;
> 3031        void __user* argp = (void __user*)arg;
> 3032        unsigned int ifindex, carrier;
> 3033        struct ifreq ifr;
> 3034        kuid_t owner;
> 3035        kgid_t group;
> 3036        int sndbuf;
> 3037        int vnet_hdr_sz;
> 3038        int le;
> 3039        int ret;
> 3040        bool do_notify = false;
> 3041
> 3042        if (cmd == TUNSETIFF || cmd == TUNSETQUEUE |
> 3043            (_IOC_TYPE(cmd) == SOCK_IOC_TYPE && cmd != SIOCGSKNS)) {
>                     // x90:: vulnerable point::
> 3044                if (copy_from_user(&ifr, argp, ifreq_len)) // bug.
> 3045                        return -EFAULT;
> 3046        } else {
> 3047                memset(&ifr, 0, sizeof(ifr));
> 3048        }
> --
>
> -author: x90

-- 
Robert Święcki

_______________________________________________
Sent through the Full Disclosure mailing list
https://nmap.org/mailman/listinfo/fulldisclosure
Web Archives & RSS: http://seclists.org/fulldisclosure/

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