[PATCH] Sanitize printf functions

[Evgeniy Stepanov]

================
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) {                                                         \
----------------
Yury Gribov wrote:
> 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.
Now I must be missing something. What would be the cause of stack overflow?

That would slow down sprintf (and snprintf, etc) by a factor of 1.5 (roughly speaking). That may be a big hit exactly because sprintf is so slow. I'm assuming that calculation the length of the output by passing 0 as size takes about the same time as actually formatting the output.


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