[PATCH] Sanitize printf functions

Yury Gribov tetra2005 at gmail.com
Wed Jan 1 09:15:08 PST 2014



================
Comment at: asan/lit_tests/TestCases/printf-4.c:6
@@ +5,3 @@
+#include <string.h>
+#include <stdlib.h>
+int main() {
----------------
Alexander Potapenko wrote:
> Please sort the includes alphabetically (here and in other tests)
Will do.

================
Comment at: sanitizer_common/sanitizer_common_interceptors_scanf.inc:11
@@ -11,2 +10,3 @@
+// Scanf/printf implementation for use in *Sanitizer interceptors.
 // Follows http://pubs.opengroup.org/onlinepubs/9699919799/functions/fscanf.html
 // with a few common GNU extensions.
----------------
Alexander Potapenko wrote:
> How about this line? Is there a printf specification we're following?
Oh, this is a good one. I've blindly reused scanf parser but it seems that printf format has richer syntax: it has flags and precision + asterisk semantics is different. I'll fix this in next rev.

================
Comment at: sanitizer_common/sanitizer_common_interceptors_scanf.inc:323
@@ +322,3 @@
+
+  COMMON_INTERCEPTOR_READ_RANGE(ctx, format, internal_strlen(format) + 1);
+
----------------
Alexander Potapenko wrote:
> Please swap this line with the above declaration and remove the empty line below.
+1

================
Comment at: sanitizer_common/sanitizer_common_interceptors_scanf.inc:337
@@ +336,3 @@
+    if (dir.argIdx != -1) {
+      // Unsupported.
+      break;
----------------
Alexander Potapenko wrote:
> How does this comment relate to the above one?
Sorry, not quite clear. I believe we don't want to bother with positional args neither in scanf, nor in printf.

================
Comment at: sanitizer_common/sanitizer_common_interceptors_scanf.inc:350
@@ +349,3 @@
+      COMMON_INTERCEPTOR_WRITE_RANGE(ctx, argp, size);
+      // FIXME: detect overlaps with format string?
+      continue;
----------------
Alexander Potapenko wrote:
> Good point, probably worth doing.
Ok, I can do this. Note that there are actually many more checks of this sort: intersection of format with target string, intersection of format with %n destination, intersection of format with scanf argument, etc., etc. I guess errors like this should already be handled by things like FORTIFY_SOURCE pretty well.


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



More information about the llvm-commits mailing list