[PATCH] Sanitize printf functions

[Yury Gribov]

================
Comment at: lib/sanitizer_common/sanitizer_common_interceptors.inc:697
@@ +696,3 @@
+      va_copy(ar, ap);                                                         \
+      int size = REAL(vname2)(str, 0, format, ar);                             \
+      if (size >= 0) {                                                         \
----------------
Evgeniy Stepanov wrote:
> Yury Gribov wrote:
> > Evgeniy Stepanov wrote:
> > > Yury Gribov wrote:
> > > > Evgeniy Stepanov wrote:
> > > > > Unless I'm missing something, you could use REAL(strlen)(str) here instead of calling *printf twice.
> > > > Hm but str isn't necessarily a string - it's just plain array of chars. I don't think we can call strlen on it.
> > > I think *printf always end the output with a null character, and never print null in the middle, but I don't see a clear indication of that in the spec.
> > Sorry, I must be missing something... My intent with REAL(vname2) (which is actually REAL(vsnprintf)) is to estimate number of chars that will be written. 0 tells vsnprintf that it should not _write_ anything, only estimate number of chars. We could simply pass NULL instead of str and it wouldn't change anything. So why do you bother about str?
> I'm just worried that it makes *printf quite a bit heavier - we now parse the format string 3 times (2 in libc *printf and 1 in our code). Alternatively, we could do 1 + 1, and then figure out the length of the resulting string with strlen() after the REAL *printf.
> 
Agreed but then we may get stack overflow before Asan is able to react. Looks like a trade-off between speed and security. Does libsanitizer have general preference for any of those?

IMHO sprintf is already so slow that we may not need to bother so much.


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