Messages in this thread | | | Date | Thu, 12 Sep 2013 00:31:12 -0700 | Subject | Re: [PATCH] vsprintf: drop comment claiming %n is ignored | From | Kees Cook <> |
| |
On Thu, Sep 12, 2013 at 12:03 AM, Jan Beulich <JBeulich@suse.com> wrote: >>>> On 11.09.13 at 22:18, Kees Cook <keescook@chromium.org> wrote: >> On Wed, Sep 11, 2013 at 1:06 PM, Joe Perches <joe@perches.com> wrote: >>> On Wed, 2013-09-11 at 12:30 -0700, Kees Cook wrote: >>>> The %n format is not ignored, so remove the incorrect comment about it. >>> >>> I think it may be better to reimplement the ignoring. >> >> Yeah, just had a quick look, and scanf doesn't use this code at all. >> I'd much rather remove %n again instead. > > Why would you want to artificially make the function diverge > from the spec? People shouldn't be caught by surprises if at all > possible, and one can certainly not expect people to go look at > the comment before the function implementation to find out > what basic (standard) features _do not_ work (one can expect > so when trying to find out about _extensions_).
The spec, in this case, is considered harmful. Without %n, format string flaws are at worst "just" information leaks. Being able to downgrade potential arbitrary memory writing vulnerabilities into info leak vulnerabilities would be a very nice improvement. As another point of reference, even Android's libc replacement, bionic, doesn't implement %n for the very same reasons.
Since there are so few users of %n in the kernel, it seems that this goal is not totally outside the realm of possibility in the short-term. The inclusion of the WARN_ON() was suggested for the very reason of helping a potential user of %n to notice that it's not supported.
-Kees
-- Kees Cook Chrome OS Security
| |