[PATCH] Sanitize printf functions

[Yury Gribov]

================
Comment at: lib/sanitizer_common/sanitizer_common_interceptors_scanf.inc:90
@@ -78,2 +89,3 @@
+      ++p;
     if (*p >= '0' && *p <= '9') {
       p = parse_number(p, &dir->fieldWidth);
----------------
Evgeniy Stepanov wrote:
> Yury Gribov wrote:
> > Evgeniy Stepanov wrote:
> > > According to the manual, field width can be "*", too. The number-or-star parsing logic could be factored out in a separate function.
> > > 
> > > According to the manual, field width can be "*", too
> > 
> > Yup, this is handled by dir->suppressed above)
> > 
> > > The number-or-star parsing logic could be factored out in a separate function.
> > 
> > Agreed.
> Oh, right. This is a bit confusing, could you add a separate field to FormatDirective instead of reusing "suppressed" here?
> 
Already done. I've also decided to split format_parse_next to {printf,scanf}_parse_next because they are different enough.

================
Comment at: lib/sanitizer_common/sanitizer_common_interceptors_scanf.inc:98
@@ +97,3 @@
+      if (output)
+        dir->printErrno = true;
+      else
----------------
Evgeniy Stepanov wrote:
> Yury Gribov wrote:
> > Yury Gribov wrote:
> > > Evgeniy Stepanov wrote:
> > > > This does not look right. %m in printf is a complete conversion specifier, it should end parsing of the current directive. And it seems like "m" could go into convSpecifier, and printErrno could be removed.
> > > > 
> > > > %m in printf is a complete conversion specifier, it should end parsing of the current directive
> > > 
> > > I agree that we are currently parsing a wider format language than really necessary. But same is true for %n in original scanf implementation and we didn't bother handling it.
> > > And it seems like "m" could go into convSpecifier, and printErrno could be removed.
> > 
> > Agreed to this one, will fix.
> What's wrong with %n? We parse it as convSpecifier which can not have anything going after it.
> It seems like simply changing the code here to
> 
> if (*p == 'm' && !output) {
>   dir->allocate=true;
>   ++p;
> }
> 
> would do the trick.
> 
Ok, should be fixed by splitting format_parse_next mentioned above.


http://llvm-reviews.chandlerc.com/D2480