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

[Pavel Machek <pavel@ucw.cz>]

On Mon 2014-10-27 11:11:53, Geert Uytterhoeven wrote:
> On Mon, Oct 27, 2014 at 10:56 AM, Pavel Machek <pavel@ucw.cz> wrote:
> > On Wed 2014-10-22 16:43:10, Kees Cook wrote:
> >> On Wed, Oct 22, 2014 at 4:26 PM, Andrew Morton
> >> <akpm@linux-foundation.org> wrote:
> >> > On Tue, 21 Oct 2014 13:21:37 -0700 Kees Cook <keescook@chromium.org> wrote:
> >> >
> >> >> From: Paul Wise <pabs3@bonedaddy.net>
> >> >>
> >> >> This partially mitigates a common strategy used by attackers for hiding
> >> >> the full contents of strings in procfs from naive sysadmins who use cat,
> >> >> more or sysctl to inspect the contents of strings in procfs.
> >> >>
> >> >> ...
> >> >>
> >> >> --- a/kernel/sysctl.c
> >> >> +++ b/kernel/sysctl.c
> >> >> @@ -1739,7 +1739,7 @@ static int _proc_do_string(char *data, int maxlen, int write,
> >> >>               while ((p - buffer) < *lenp && len < maxlen - 1) {
> >> >>                       if (get_user(c, p++))
> >> >>                               return -EFAULT;
> >> >> -                     if (c == 0 || c == '\n')
> >> >> +                     if (c == 0 || c == '\n' || c == '\r')
> >> >>                               break;
> >> >>                       data[len++] = c;
> >> >>               }
> >> >
> >> > There are no valid uses of \r in a procfs write?
> >>
> >> I struggle to imagine one; everything I found that uses proc_dostring
> >> seems to be names, paths, and commands.
> >
> > Well, filename can contain \r, right?
> 
> Even \n. AFAIK, the only thing a filename cannot contain is the nul character
> and a forward slash, as that's used to separate path components (so slash
> is valid for a path name).
> 
> Still, we can hide stuff using ANSI ESC sequences, and e.g. backspaces,
> right?

Right :-(. So this patch makes no sense, it would have to forbid any
character < 32 to be effective.

And more sfun is... with ESCape sequences, you can actually cause
stuff to be forced as an input on poor admin's terminal. cat could be
"fixed", but pretty much all the command line tools could be abused to
do this... I see no reasonable way forward.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html