Hi, wt., 15 cze 2021 o 09:56 KJ Jung <x90cx9...@gmail.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/