Re: [PATCH 3/3] vsprintf: ignore %n again

[Kees Cook]

On Tue, Oct 8, 2013 at 1:28 PM, Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
> On Mon,  7 Oct 2013 19:56:51 -0700 Kees Cook <keescook@xxxxxxxxxxxx> wrote:
>
>> This ignores %n in printf again, as was originally documented. Implementing
>> %n poses a greater security risk than utility, so it should stay ignored.
>> To help anyone attempting to use %n, a warning will be emitted if it is
>> encountered.
>>
>> Based on earlier patch by Joe Perches.
>
> Well this sucks.  Nowhere in this patchset are we told what is the
> alleged security risk with %n.  There's even a runtime warning telling
> people not to use it, but we've provided no way for them to find out
> *why*.
>
> Please send along suitable changelog text so I can fix this up.

Perhaps add these two paragraphs to the end of the "vsprintf: ignore
%n again" commit:


Because %n was designed to write to pointers on the stack, it has been
frequently used as an attack vector when bugs are found that leak
user-controlled strings into functions that ultimately process format
strings. While this class of bug can still be turned into an
information leak, removing %n eliminates the common method of
elevating such a bug into an arbitrary kernel memory writing
primitive, significantly reducing the danger of this class of bug.

For seq_file users that need to know the length of a written string
for padding, please see seq_setwidth() and seq_pad() instead.


> A new checkpatch rule might be appropriate?

I can look into that -- I worry it won't be very effective since
checkpatch lacks the knowledge of which functions are taking format
strings, and looking for just %n may lead to some false positives.
Maybe I can look for the common case of %n" (at the end of a string
literal).

> Two of these patches were acked-by:you.  But you sent the patches, so I
> changed these to Signed-off-by:, as per
> Documentation/SubmittingPatches, section 12.

Ah! Yes, thanks for fixing that.

-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/