[PATCH] Sanitize printf functions

[Yury Gribov]

================
Comment at: sanitizer_common/sanitizer_common_interceptors_scanf.inc:10
@@ -9,3 +9,3 @@
 //
 // Scanf implementation for use in *Sanitizer interceptors.
 // Follows http://pubs.opengroup.org/onlinepubs/9699919799/functions/fscanf.html
----------------
Alexander Potapenko wrote:
> Please update this comment.
> Also, how about renaming the file to something like "sanitizer_common_interceptors_printf_scanf.inc" or "sanitizer_common_interceptors_format.inc"?
> Please update this comment.

Will do.

> Also, how about renaming the file to something like "sanitizer_common_interceptors_printf_scanf.inc"

But won't this make diff unreadable? Perhaps this should be a separate patch?

================
Comment at: sanitizer_common/sanitizer_common_interceptors_scanf.inc:281
@@ -280,1 +280,3 @@
 
+  // TODO(y.gribov): check format?
+
----------------
Alexander Potapenko wrote:
> Yury Gribov wrote:
> > Kostya Serebryany wrote:
> > > good idea. feel free to implement or to leave the TODO.
> > > Actually this code base uses FIXME (w/o name) more often. 
> > > Actually this code base uses FIXME (w/o name) more often.
> > 
> > Got it. Anyway, this was more a question for reviewers than real TODO. I'll add format check in next rev.
> We sure need to check |format|. Since you've already touched scanf_common in this CL, why not just add
>   COMMON_INTERCEPTOR_READ_RANGE(ctx, format, internal_strlen(format) + 1);
> 
> ?
Yup, already added this in new version of the patch. I didn't resubmit it because I wanted to gather reviews from all team members.

The bigger question is whether we want to check size of input buffer in sscanf and friends.

BTW what's the general rule: should I resubmit patch as often as possible or wait till everyone comments?

================
Comment at: sanitizer_common/sanitizer_common_interceptors_scanf.inc:362
@@ +361,3 @@
+      default:
+        break; // TODO(ygribov): issue warning?
+      }
----------------
Alexander Potapenko wrote:
> Two spaces before the comment, please.
Do you think we need a warning here? If yes, I'll add it, if not - I'll remove the comment alltogether.

================
Comment at: sanitizer_common/sanitizer_platform_interceptors.h:76
@@ -75,3 +75,3 @@
 # define SANITIZER_INTERCEPT_SCANF SI_NOT_WINDOWS
 # define SANITIZER_INTERCEPT_ISOC99_SCANF SI_LINUX
 
----------------
Alexander Potapenko wrote:
> I wonder why the lines above have this strange indentation, but let us fix that in a separate commit.
Looks like something lint should be able to handle.


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