oss-security - Re: Potential regression and/or incomplete fix for CVE-2017-12762

Follow @Openwall on Twitter for new release announcements and other news

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200211213431.GA26083@grsecurity.net>
Date: Tue, 11 Feb 2020 16:34:31 -0500
From: Brad Spengler <spender@...ecurity.net>
To: oss-security@...ts.openwall.com
Subject: Re: Potential regression and/or incomplete fix for
 CVE-2017-12762

Hi Ibrahim,

> 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.

This isn't correct.  There is a 'count' variable that decrements to zero, yes,
but that's not what is used to index the strings.  'res' is used for that, and
it increments from zero as you'd expect.

Regarding OOB, there is the read-by-word trickery, but it's safe and won't
trip up KASAN for the max 7 bytes it can end up reading past bounds, and won't
in any instance cross a page boundary.

Since 'param' is guaranteed to be NUL-terminated from the fix (the isdn_common.c
change), so is 'p', so the strscpy is fine here, especially since the later
use of the buffer (coming from the netdev netname) uses strlen as the length
for the buffer copy to userland here:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/isdn/i4l/isdn_common.c?id=9f5af546e6acc30f075828cb58c7f09665033967#n1385
So strscpy_pad() wasn't necessary in this instance, despite it often being
needed for kernel work (and the better defensive choice, unless performance
is critical and you can guarantee the remaining part of the buffer never gets
copied to userland or used in any way).

> ## 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

This wasn't due to a revert, the fix was just never backported to 3.16. 
Happens all the time.  There's never a guarantee that just because a 
security fix is backported to some newer kernel version that it'll be 
backported to all affected versions.  If the patch doesn't apply cleanly 
and no one fixes it up, it just never gets fixed.

For this instance, you can confirm it by looking at the git log for that tree:
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/log/drivers/isdn/i4l/isdn_net.c?h=linux-3.16.y

The 3.16 kernel has a different maintainer than others listed on kernel.org:
https://www.kernel.org/category/releases.html
so there may be different critera for what's selected for backporting there.

Thanks,
-Brad

Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)

Please check out the Open Source Software Security Wiki, which is counterpart to this mailing list.

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.