On 28/01/14 10:03, Kees Cook wrote: > Now that there has been a full release of the kernel, and all users > of %n have been dropped, switch to %n use triggering a BUG. Ignoring > arguments could be used to assist in information leaks if an arbitrary > format string was under the control of an attacker.
Not sure I follow the reasoning. %n no longer does anything in the kernel, so there is no risk if it does manage to find its way into a printed string. BUG() is for unrecoverable errors, which this clearly isn't. Information leaks via injectable strings are still possible if an attacker can insert %x, %d, etc. %n is more problematic since it allows for code injection, which is why it got removed. %n is not however, required to get an infoleak via a format string, so I think the summary is also a bit misleading. ~Ryan > > Signed-off-by: Kees Cook <[email protected]> > --- > lib/vsprintf.c | 13 +++++-------- > 1 file changed, 5 insertions(+), 8 deletions(-) > > diff --git a/lib/vsprintf.c b/lib/vsprintf.c > index 185b6d300ebc..a27fd7f61325 100644 > --- a/lib/vsprintf.c > +++ b/lib/vsprintf.c > @@ -1735,15 +1735,12 @@ int vsnprintf(char *buf, size_t size, const char > *fmt, va_list args) > case FORMAT_TYPE_NRCHARS: { > /* > * Since %n poses a greater security risk than > - * utility, ignore %n and skip its argument. > + * utility, it should not be implemented. Instead, > + * BUG when encountering %n, since there are no > + * legitimate users and skipping arguments could > + * assist information leak attacks. > */ > - void *skip_arg; > - > - WARN_ONCE(1, "Please remove ignored %%n in '%s'\n", > - old_fmt); > - > - skip_arg = va_arg(args, void *); > - break; > + BUG(); > } > > default: > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
