Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Sat, 25 Jun 2011 22:52:34 +0200
From: Willy Tarreau <w@....eu>
To: Vasiliy Kulikov <segoon@...nwall.com>
Cc: Alan Cox <alan@...rguk.ukuu.org.uk>, security@...nel.org,
        kernel-hardening@...ts.openwall.com, Namhyung Kim <namhyung@...il.com>,
        Greg Kroah-Hartman <gregkh@...e.de>, James Morris <jmorris@...ei.org>,
        linux-kernel@...r.kernel.org, Joe Perches <joe@...ches.com>,
        Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [Security] [PATCH] kernel: escape non-ASCII and control characters in printk()

Hi Vasiliy,

On Wed, Jun 22, 2011 at 11:07:39PM +0400, Vasiliy Kulikov wrote:
> The final check should be:
> 
>     if (iscntrl(c) && (c != '\n') && (c != '\t'))
> 
> Any comments against this variant?

In fact, I'm not sure we're adding that much protection with such a check
because as long as the '\n' is allowed, it's easy to fake logs. For instance :

$ cd /tmp
$ echo "main() { *(int*)0=0; }" | gcc -xc -o fail -
$ ln -s fail $'Oops: 000\nklogd'
$ ./Oops*
$ dmesg|tail -2
Oops: 000
klogd[1927]: segfault at 0 ip 0000000008048337 sp 00000000ffb54ba4 error 6 in fail[8048000+1000]
$

In an ideal world, only \n should be escaped since it's the only delimitor,
and klogd would get the raw logs with lines correctly sequenced. Other
characters should probably be escaped before going to log files if those
files are supposed to be readable on a terminal.

But I recall it was not possible to escape \n when we worked on the subject
several years ago on 2.4, because some drivers used to send multi-line logs
in a single printk().

The fundamental issue we're facing is that neither inputs nor outputs have
been clearly typed in the past. I tend to consider that a log file is readable
by "tail -f" and a such should not contain dangerous chars, however I also
tend to prefer sending raw logs over the network when they are archived by
different means. In the end it makes sense for the kernel and klogd to
exchange raw logs and syslogd should encode them when pushing them to a
file.

Best regards,
Willy

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.