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)
Powered by blists - more mailing lists
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.