[PATCH] Sanitize printf functions
Evgeniy Stepanov
eugenis at google.com
Thu Jan 9 05:40:59 PST 2014
================
Comment at: lib/sanitizer_common/sanitizer_common_interceptors_scanf.inc:360
@@ +359,3 @@
+
+#define skip_scalar_arg(aq, convSpec, size) \
+ do { \
----------------
Does it really need to be a macro?
================
Comment at: lib/sanitizer_common/sanitizer_common_interceptors_scanf.inc:98
@@ +97,3 @@
+ if (output)
+ dir->printErrno = true;
+ else
----------------
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.
================
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);
----------------
According to the manual, field width can be "*", too. The number-or-star parsing logic could be factored out in a separate function.
================
Comment at: lib/sanitizer_common/sanitizer_common_interceptors_scanf.inc:168
@@ -129,3 +167,3 @@
++p;
- } else if (*p == '[') {
+ } else if (!output && *p == '[') {
// Watch for %a[h-j%d], if % appears in the
----------------
At this point allowGnuMalloc is true. It does not make sense for output to be true. Please remove this and instead check this invariant at the beginning of the functions (!(allowGnuMalloc && output)).
================
Comment at: lib/sanitizer_common/sanitizer_common_interceptors_scanf.inc:293
@@ +292,3 @@
+ return FSS_INVALID;
+ if (output || dir->fieldWidth == 0)
+ return FSS_STRLEN;
----------------
Are these guys ([...]) even allowed in printf?
http://llvm-reviews.chandlerc.com/D2480
More information about the llvm-commits
mailing list