Re: [PATCH] sysctl: terminate strings also on \r

[Kees Cook]

On Thu, Oct 23, 2014 at 11:23 AM, Andrew Morton
<akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
> On Thu, 23 Oct 2014 09:39:09 -0700 Kees Cook <keescook@xxxxxxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/