oss-sec mailing list archives

Potential regression and/or incomplete fix for CVE-2017-12762


From: Ibrahim el-sayed <i.elsayed92 () gmail com>
Date: Tue, 11 Feb 2020 19:27:18 +0000

Hello,
I stumbled upon CVE-2017-12762 which has a CVSS score of 10 and I think the
patch is incomplete and it might have regressed in the stable version. I am
probably wrong hence this email to see if anyone familiar with this CVE and
the fix and tell me if I am wrong.

## Incomplete patch
The patch can be found in
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=9f5af546e6acc30f075828cb58c7f09665033967


This is the vulnerable piece of code
```
char *
isdn_net_newslave(char *parm)
{
    char *p = strchr(parm, ',');
    isdn_net_dev *n;
    char newname[10];

    if (p) {
        /* Slave-Name MUST not be empty or overflow 'newname' */
        *if (strscpy(newname, p + 1, sizeof(newname)) <= 0)*
            return NULL;
        *p = 0;
        /* Master must already exist */
        if (!(n = isdn_net_findif(parm)))
            return NULL;
        /* Master must be a real interface, not a slave */
        if (n->local->master)
            return NULL;
        /* Master must not be started yet */
        if (isdn_net_device_started(n))
            return NULL;
        return (isdn_net_new(newname, n->dev));
    }
    return NULL;
}
```

I think it is incomplete and can lead to reading out of bound since it does
*not* check if the src buffer (p) in this case has 10 bytes at least. The
fix assumes p has 10 bytes and copies that into newname. The fix
uses strscpy (
https://github.com/torvalds/linux/blob/cc12071ff39060fc2e47c58b43e249fe0d0061ee/lib/string.c#L180)
which
based on its code it starts copying from count and decrements to zero.
which in this case if param is a string similar to "aa,", then p will point
to the last byte of the string, strscpy will copy from p+1+sizeof(newname)
= p+1+10 -[to]-> p+1 which would allow reading 10 bytes after the buffer p


I do not know if param can end with "," or if there is any validation
checks that it does not end with "," hence this might not be a bug but only
bad practice.


## Regression
I looked quickly into latest version for the kernel v3.16.81 and it seems
that the patch was probably reverted as the code matches exactly to the
vulnerable version to the CVE (
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/isdn/i4l/isdn_net.c?id=v3.16.81#n2646
)
Not sure if the fix was reworked but wanted to surface that issue as well



Ibrahim

Current thread: