Messages in this thread |  | | Date | Thu, 23 Oct 2014 11:50:48 -0700 | Subject | Re: [PATCH] sysctl: terminate strings also on \r | From | Kees Cook <> |
| |
On Thu, Oct 23, 2014 at 11:23 AM, Andrew Morton <akpm@linux-foundation.org> wrote: > On Thu, 23 Oct 2014 09:39:09 -0700 Kees Cook <keescook@chromium.org> wrote: > >> > I wonder if the chances of damage would be lower if we were to continue >> > to accept the \r, but turn it into something else ("\r"?) when it is >> > read. >> >> I think that would complicate things more than help them. > > Why.
My thinking is that currently we're only aware of \r being used to assist an attacker. If we "escape" \r's in our output, we might run the risk of shells interpreting things differently than expected, etc. Right now, both the "cat" of a sysctl, and the internal use of the string (for say, a command line) just reads directly from the string memory. If we add escaping to that on output, we're adding complication to this system that I think exceeds the utility of the protection.
More simply, I would rather leave \r as-is than introduce an escaping mechanism for just \r. If there were other cases of equally problematic inputs, there may be a benefit, but for just \r I think adding complexity to the sysctl code would have a net negative result.
>> If there's a >> legit use of \r, I'll let Paul Wise debate how to proceed. :) > > I don't know who Paul is. Please take this seriously; we don't want to > have to revert this after breaking a bunch of people's setups.
Paul is the author of the patch I forwarded. I am taking it seriously -- the only case I can come up with for \r after continuing to ponder this is that someone has done really strange things with a command-line or core dump pattern. Disallowing \r would break that case, but I cannot come up with any reason why someone would attempt to inject \r into core files or command lines via sysctl.
If there is a legit use for \r, then we can ignore this patch entirely. It seemed reasonable to me to send it upstream since the only use I (and others) have seen is to obfuscate attacks. If it's an easy fix, let's do it. If it's going to break people or add code complexity, I don't want it.
-Kees
-- Kees Cook Chrome OS Security
|  |