libsanitizer merge from upstream r173241

Jakub Jelinek jakub@redhat.com
Wed Jan 23 12:39:00 GMT 2013


On Wed, Jan 23, 2013 at 04:24:01PM +0400, Evgeniy Stepanov wrote:
> >  So, e.g. whenever match_spec
> > > returns 0, it should break out of the loop, rather than continue.
> > > And for %hh it doesn't check following letters, no match_spec at all.
> 
> That's cause they don't change the write size. The same for %h, the
> following letters don't matter to us. It seems safe to "continue" as
> soon as we are sure we know what the current spec's write length is.

You can't know.  What if say %hhQ is seen where Q is a user spec defined
through some future hook?  For printf you can already add things like
that, and say let the va_arg argument be a pointer to some struct
(some people e.g. print struct sockaddr* that way etc.).  What if it takes
more than one pointer.  IMHO you really need to parse the whole directive,
and only if you detect no errors, check the corresponding pointer (unless
%*), if you detect any errors or unknown things, just give up immediately.

> > > %[, %s, %c, %n, %ls, %lc, %p, %A and many others aren't handled FYI (again, not
> > > a problem if the implementation is conservative).  Handling %c, %s and
> > > %[0123456789] style would be worthwhile though, I bet most of the buffer
> > > overflows are from the use of those scanf specifiers.
> 
> At least some of those are handled (%p, %n). Not sure what %A is, it's
> not in my man page.

http://pubs.opengroup.org/onlinepubs/9699919799/functions/fscanf.html

> > > BTW, %as, %aS and %a[ should always result in giving up, as there is an
> > > ambiguity between GNU extensions and C99/POSIX and libasan can't know how
> > > glibc resolves that ambiguity.
> > >
> > > What if glibc adds a scanf hook (like it has already printf hooks), apps
> > > could then register their own stuff and the above would then break.  It
> > > really should be very conservative, and should be checked e.g. with all
> > > glibc's *scanf tests (e.g. stdio-common/scanf[0-9]*.c,
> > > stdio-common/tst-sscanf.c).
> 
> I'll add support for the missing %specs. About the testing, these
> files seem like GPL, so I'd prefer not to look at them at all. We've
> got a smallish test for the scanf implementation in sanitizer_common,
> but it would be really great to run it on full glibc scanf tests.
> Would you be willing to setup such testing gnu-side?

If you mail me (or check in?) some fixes for the above mentioned issues, I
can do some testing.  But right now I'm pretty sure it would just fail.

	Jakub



More information about the Gcc-patches mailing list