steakhal created this revision.
steakhal added reviewers: NoQ, xazax.hun, donat.nagy, Szelethus.
Herald added subscribers: manas, ASDenysPetrov, martong, dkrupp,
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware.
Herald added a project: All.
steakhal requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Previously, we didn't report OOB accesses if the pointer itself was
tainted. This looks weird, but there is weird code out there, code like
inside the Juliet benchmark.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D159105
Files:
clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
clang/test/Analysis/taint-generic.c
Index: clang/test/Analysis/taint-generic.c
===================================================================
--- clang/test/Analysis/taint-generic.c
+++ clang/test/Analysis/taint-generic.c
@@ -55,10 +55,19 @@
// CHECK-INVALID-ARG-SAME: that expects an argument number for propagation
// CHECK-INVALID-ARG-SAME: rules greater or equal to -1
+typedef typeof(sizeof(int)) size_t;
typedef long long rsize_t;
void clang_analyzer_isTainted_char(char);
void clang_analyzer_isTainted_charp(char*);
void clang_analyzer_isTainted_int(int);
+void clang_analyzer_isTainted_int_ptr(const int *);
+size_t clang_analyzer_getExtent(const void *);
+void clang_analyzer_dump_int(int);
+void clang_analyzer_dump_int_ptr(const int *);
+void clang_analyzer_dump_char(char);
+void clang_analyzer_dump_char_ptr(const char*);
+void clang_analyzer_value(char);
+void clang_analyzer_printState();
int scanf(const char *restrict format, ...);
char *gets(char *str);
@@ -1107,3 +1116,36 @@
setproctitle_init(1, argv, 0); // expected-warning {{Untrusted data is passed to a user-defined sink}}
setproctitle_init(1, real_argv, argv); // expected-warning {{Untrusted data is passed to a user-defined sink}}
}
+
+void testTaintedPtr() {
+ // Read a pointer from a tainted source and dereference it.
+ int *ptr;
+ scanf("%ld", &ptr); // expected-warning {{format specifies type 'long *' but the argument has type 'int **'}}
+ *ptr = 44; // expected-warning {{Out of bound memory access (index is tainted)}}
+}
+
+void testTaintedUnconstrainedIndex() {
+ int n;
+ scanf("%d", &n);
+ Buffer[n] = 1; // expected-warning {{Out of bound memory access (index is tainted)}}
+}
+
+void testTaintedLowerConstrainedIndex() {
+ int n;
+ scanf("%d", &n);
+ if (n >= 0) {
+ // We only constained the lower end, and it's tainted => report.
+ Buffer[n] = 1; // expected-warning {{Out of bound memory access (index is tainted)}}
+ }
+}
+
+void testUnknownExtentWithTaintedIndex(void) {
+ extern void v;
+ int *p = (int *)&v;
+ int n;
+ scanf("%d", &n);
+
+ clang_analyzer_isTainted_int(n); // expected-warning {{YES}}
+ clang_analyzer_dump_int(clang_analyzer_getExtent(p)); // expected-warning {{Unknown}}
+ p[n] = 42; // expected-warning {{Out of bound memory access (index is tainted)}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
+++ clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
@@ -193,6 +193,11 @@
reportOOB(checkerContext, state_precedesLowerBound, OOB_Precedes);
return;
}
+ if (state_precedesLowerBound && state_withinLowerBound &&
+ isTainted(state, location)) {
+ reportTaintOOB(checkerContext, state_precedesLowerBound, location);
+ return;
+ }
if (state_withinLowerBound)
state = state_withinLowerBound;
@@ -204,17 +209,16 @@
auto [state_withinUpperBound, state_exceedsUpperBound] =
compareValueToThreshold(state, ByteOffset, *KnownSize, svalBuilder);
- if (state_exceedsUpperBound) {
- if (!state_withinUpperBound) {
- // We know that the index definitely exceeds the upper bound.
- reportOOB(checkerContext, state_exceedsUpperBound, OOB_Excedes);
- return;
- }
- if (isTainted(state, ByteOffset)) {
- // Both cases are possible, but the index is tainted, so report.
- reportTaintOOB(checkerContext, state_exceedsUpperBound, ByteOffset);
- return;
- }
+ if (state_exceedsUpperBound && !state_withinUpperBound) {
+ // We know that the index definitely exceeds the upper bound.
+ reportOOB(checkerContext, state_exceedsUpperBound, OOB_Excedes);
+ return;
+ }
+
+ if (state_withinUpperBound && state_exceedsUpperBound &&
+ isTainted(state, location)) {
+ reportTaintOOB(checkerContext, state_exceedsUpperBound, location);
+ return;
}
if (state_withinUpperBound)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
