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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 23 Oct 2014 11:50:48 -0700
From:	Kees Cook <keescook@...omium.org>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	LKML <linux-kernel@...r.kernel.org>,
	Paul Wise <pabs3@...edaddy.net>,
	Ingo Molnar <mingo@...nel.org>, Rik van Riel <riel@...hat.com>,
	David Rientjes <rientjes@...gle.com>,
	Aaron Tomlin <atomlin@...hat.com>,
	Dario Faggioli <raistlin@...ux.it>,
	Andi Kleen <ak@...ux.intel.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Jens Axboe <axboe@...com>,
	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
Subject: Re: [PATCH] sysctl: terminate strings also on \r

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