oss-sec mailing list archives
Re: Potential regression and/or incomplete fix for CVE-2017-12762
From: Ibrahim el-sayed <i.elsayed92 () gmail com>
Date: Fri, 14 Feb 2020 10:47:16 +0000
Hi Brad, Thank you very much for your reply. This actually clarifies everything :) Ibrahim On Tue, Feb 11, 2020 at 9:37 PM Brad Spengler <spender () grsecurity net> wrote:
Hi Ibrahim,I think it is incomplete and can lead to reading out of bound since itdoes*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 wellThis 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
-- Regards Ibrahim M. El-Sayed Security Engineer Website: https://www.ibrahim-elsayed.com @ibrahim_mosaad
Current thread:
- Potential regression and/or incomplete fix for CVE-2017-12762 Ibrahim el-sayed (Feb 11)
- Re: Potential regression and/or incomplete fix for CVE-2017-12762 Brad Spengler (Feb 11)
- Re: Potential regression and/or incomplete fix for CVE-2017-12762 Ibrahim el-sayed (Feb 14)
- Re: Potential regression and/or incomplete fix for CVE-2017-12762 Brad Spengler (Feb 11)