Re: printk()s of user-supplied strings (Re: [PATCH] binfmt_elf.c : the BAD_ADDR macro again)

[Solar Designer <solar@openwall.com>]

On Thu, Aug 24, 2006 at 06:46:33PM +0200, Willy Tarreau wrote:
> On Thu, Aug 24, 2006 at 08:44:25PM +0400, Solar Designer wrote:
> > Yet there are lots of such printk()s - and I suggest that we make a
> > determination on all of them before we possibly start fixing individual
> > ones.  In fact, perhaps there are too many of them to be fixing any in
> > 2.4, unless we determine to somehow harden printk() itself.
> > 
> > Even current->comm is untrusted user input, but there are at least 58
> > printk()s of it in 2.4.33.  (58 is the number spotted by a grep that
> > would only match those that have current->comm on the same line with
> > printk itself.)
> 
> I still fail to imagine a useful case for printk("%s") to output non-printable
> chars verbatim. I really think that the right solution would be for printk()
> to output all non-printable chars escaped (eg: \n, \r, \xXX).

Well, this would be ASCII codes 0 through 0x1f and 0x7f through 0x9f.
Unfortunately, some of the latter correspond to national letters and
other visible characters with weird charsets:

	http://en.wikipedia.org/wiki/Windows_code_page

but perhaps it is OK to not support them in kernel logs - syslogd's
printline() would escape them anyway, so we can do it earlier to also
protect the console.

Would you escape backslashes themselves, though?  Perhaps not.  syslogd
(as maintained in Linux sysklogd) doesn't.  Yes, this escaping is not
reliably reversible in that case.

> It would
> definitely fix the problem without removing useful information. I remember
> having had the problem a long time ago with a name extracted from a string
> supplied by the BIOS which mangled the dmesg at early boot.
> 
> It would be too much work (and too risky) to expect from all drivers to check
> their strings for correctness, so the hardening way would be the best one IMHO.

I agree.

Alexander