Date: Tue, 19 Sep 2017 17:10:53 -0400 From: Daniel Micay <danielmicay@...il.com> To: Solar Designer <solar@...nwall.com>, riel@...hat.com Cc: linux-kernel@...r.kernel.org, tytso@....edu, keescook@...omium.org, hpa@...or.com, luto@...capital.net, mingo@...nel.org, x86@...nel.org, linux-arm-kernel@...ts.infradead.org, catalin.marinas@....com, linux-sh@...r.kernel.org, ysato@...rs.sourceforge.jp, kernel-hardening@...ts.openwall.com Subject: Re: [PATCH v2 0/5] stackprotector: ascii armor the stack canary > Brad trolls us all lightly with this trivia question: > > https://twitter.com/grsecurity/status/905246423591084033 I'll respond to your proposed scenario rather than guessing at what is being suggested there and if it's actually the same thing as what you've brought up. They've stated many times that stack canaries are near useless and enabling PaX UDEREF on i386 actually disabled stack canaries and presumably still does after the last public patches: + select HAVE_CC_STACKPROTECTOR if X86_64 || !PAX_MEMORY_UDEREF Making the kernel more vulnerable to remote stack overflow exploits like this month's overhyped Bluetooth bug to improve security against local exploits seems like much more of a compromise. Not that i386 has any real relevance anymore, but I think that's some interesting context... It's not like upstream has a monopoly on needing to make hard choices for security features, I don't see this as one of those hard choices like whether sacrificing availability for SIZE_OVERFLOW makes sense. > "For #trivia can you describe one scenario where this change actually > helps exploitation using similar C string funcs?" > > I suppose the expected answer is: > > The change helps exploitation when the overwriting string ends just > before the canary. Its NUL overwriting the NUL byte in the canary > would > go undetected. Before this change, there would be a 255/256 chance of > detection. > > I hope this was considered. The change might still be a good > tradeoff, > or it might not, depending on which scenarios are more likely (leak of > canary value or the string required in an exploit ending at that exact > byte location), and we probably lack such statistics. It was considered that guaranteeing some forms of string overflows are contained within the frame can have a negative impact but I don't think it's significant. There would need to be something worth overwriting between the source of the overflow and the canary. Since SSP reorders local variables within the stack frame based on risk of overflow, the targets would be locals that Clang/GCC considers to have a higher risk of overflow. An array can only have other arrays between it and the canary and there's ordering by size (small vs. large at least) within the array section. If there was a zero byte in any of that other data between the source of the overflow and the canary, this wouldn't make a difference anyway. Since the canary is only checked on return, it's also only relevant if control flow still makes it to the function epilogue and it still matters after the attacker has accomplished their goal. The chance of there being a zero as a leading byte is already 1/256, but there could also be a NUL elsewhere in the canary compromising a subset of the entropy for this kind of scenario too so it's a tiny bit higher than a 1/256 chance already. Not much resistance to brute force if it really does ever end up mattering, unless canaries with NUL in leading positions were actually *rejected*... There's a more substantial compromise between zero and non-zero poison on free but for production use. This was part of a set of changes where CopperheadOS switched from non-zero fill -> zero fill on free for the kernel and userspace and added a leading zero byte on 64-bit for heap and stack canaries in the production builds while keeping around the old way of doing things for some forms of debug builds. Non-zero fill on free ended up causing too much harm by turning benign bugs into bugs that might have been exploitable even beyond DoS in some cases tied to non-NUL-terminated strings where non-zero poisoning got rid of zeroes that were otherwise all over the heap containing them or when it broke code wrongly depending on uninitialized data being zero in practice. I think there's more of an argument to be had about poison-on-free as a mitigation. This leading zero byte? Not so much, beyond perhaps wanting the option to turn it off for bug finding... but there are better tools for that now. > I am not proposing to revert the change. I had actually contemplated > speaking up when this was discussed, but did not for lack of a better > suggestion. We could put/require a NUL in the middle of the canary, > but > with the full canary being only 64-bit at most that would also make > some > attacks easier. > > So this is JFYI. No action needed on it, I think. It might make sense to put it in the middle, but a scenario where it matters seems unlikely and as you mention it would make it weaker it some other cases. I think it's already the best choice right now. I'd choose XOR canaries over a leading zero byte if it could only be one or the other, but I'm not really convinced that there's any significant loss compared to the normal canaries.
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.