From b8ae323cca61dc1edcd36e9ae18c7e4c3d76d52e Mon Sep 17 00:00:00 2001 From: Balazs Benics Date: Mon, 14 Feb 2022 18:44:30 +0100 Subject: [PATCH] Revert "[analyzer] Add failing test case demonstrating buggy taint propagation" This reverts commit 744745ae195f0997e5bfd5aa2de47b9ea156b6a6. I'm reverting this since this patch caused a build breakage. https://lab.llvm.org/buildbot/#/builders/91/builds/3818 --- .../Checkers/GenericTaintChecker.cpp | 25 ++----------- .../taint-checker-callback-order-has-definition.c | 42 ---------------------- ...int-checker-callback-order-without-definition.c | 34 ------------------ 3 files changed, 3 insertions(+), 98 deletions(-) delete mode 100644 clang/test/Analysis/taint-checker-callback-order-has-definition.c delete mode 100644 clang/test/Analysis/taint-checker-callback-order-without-definition.c diff --git a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp index 428778e..e2209e3 100644 --- a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp @@ -32,8 +32,6 @@ #include #include -#define DEBUG_TYPE "taint-checker" - using namespace clang; using namespace ento; using namespace taint; @@ -693,13 +691,6 @@ void GenericTaintChecker::checkPostCall(const CallEvent &Call, if (TaintArgs.isEmpty()) return; - LLVM_DEBUG(for (ArgIdxTy I - : TaintArgs) { - llvm::dbgs() << "PostCall<"; - Call.dump(llvm::dbgs()); - llvm::dbgs() << "> actually wants to taint arg index: " << I << '\n'; - }); - for (ArgIdxTy ArgNum : TaintArgs) { // Special handling for the tainted return value. if (ArgNum == ReturnValueIndex) { @@ -777,25 +768,15 @@ void GenericTaintRule::process(const GenericTaintChecker &Checker, /// Propagate taint where it is necessary. ForEachCallArg( - [this, &State, WouldEscape, &Call](ArgIdxTy I, const Expr *E, SVal V) { - if (PropDstArgs.contains(I)) { - LLVM_DEBUG(llvm::dbgs() << "PreCall<"; Call.dump(llvm::dbgs()); - llvm::dbgs() - << "> prepares tainting arg index: " << I << '\n';); + [this, &State, WouldEscape](ArgIdxTy I, const Expr *E, SVal V) { + if (PropDstArgs.contains(I)) State = State->add(I); - } // TODO: We should traverse all reachable memory regions via the // escaping parameter. Instead of doing that we simply mark only the // referred memory region as tainted. - if (WouldEscape(V, E->getType())) { - LLVM_DEBUG(if (!State->contains(I)) { - llvm::dbgs() << "PreCall<"; - Call.dump(llvm::dbgs()); - llvm::dbgs() << "> prepares tainting arg index: " << I << '\n'; - }); + if (WouldEscape(V, E->getType())) State = State->add(I); - } }); C.addTransition(State); diff --git a/clang/test/Analysis/taint-checker-callback-order-has-definition.c b/clang/test/Analysis/taint-checker-callback-order-has-definition.c deleted file mode 100644 index 295f95c..0000000 --- a/clang/test/Analysis/taint-checker-callback-order-has-definition.c +++ /dev/null @@ -1,42 +0,0 @@ -// RUN: %clang_analyze_cc1 %s \ -// RUN: -analyzer-checker=core,alpha.security.taint \ -// RUN: -mllvm -debug-only=taint-checker \ -// RUN: 2>&1 | FileCheck %s - -// FIXME: We should not crash. -// XFAIL: * - -struct _IO_FILE; -typedef struct _IO_FILE FILE; -FILE *fopen(const char *fname, const char *mode); - -void nested_call(void) {} - -char *fgets(char *s, int n, FILE *fp) { - nested_call(); // no-crash: we should not try adding taint to a non-existent argument. - return (char *)0; -} - -void top(const char *fname, char *buf) { - FILE *fp = fopen(fname, "r"); - // CHECK: PreCall prepares tainting arg index: -1 - // CHECK-NEXT: PostCall actually wants to taint arg index: -1 - - if (!fp) - return; - - (void)fgets(buf, 42, fp); // Trigger taint propagation. - // CHECK-NEXT: PreCall prepares tainting arg index: -1 - // CHECK-NEXT: PreCall prepares tainting arg index: 0 - // CHECK-NEXT: PreCall prepares tainting arg index: 1 - // CHECK-NEXT: PreCall prepares tainting arg index: 2 - - // FIXME: We should propagate taint from PreCall -> PostCall. - // CHECK-NEXT: PostCall actually wants to taint arg index: -1 - // CHECK-NEXT: PostCall actually wants to taint arg index: 0 - // CHECK-NEXT: PostCall actually wants to taint arg index: 1 - // CHECK-NEXT: PostCall actually wants to taint arg index: 2 - - // FIXME: We should not crash. - // CHECK: PLEASE submit a bug report -} diff --git a/clang/test/Analysis/taint-checker-callback-order-without-definition.c b/clang/test/Analysis/taint-checker-callback-order-without-definition.c deleted file mode 100644 index 962e8b2..0000000 --- a/clang/test/Analysis/taint-checker-callback-order-without-definition.c +++ /dev/null @@ -1,34 +0,0 @@ -// RUN: %clang_analyze_cc1 %s \ -// RUN: -analyzer-checker=core,alpha.security.taint \ -// RUN: -mllvm -debug-only=taint-checker \ -// RUN: 2>&1 | FileCheck %s - -struct _IO_FILE; -typedef struct _IO_FILE FILE; -FILE *fopen(const char *fname, const char *mode); - -char *fgets(char *s, int n, FILE *fp); // no-definition - -void top(const char *fname, char *buf) { - FILE *fp = fopen(fname, "r"); // Introduce taint. - // CHECK: PreCall prepares tainting arg index: -1 - // CHECK-NEXT: PostCall actually wants to taint arg index: -1 - - if (!fp) - return; - - (void)fgets(buf, 42, fp); // Trigger taint propagation. - - // FIXME: Why is the arg index 1 prepared for taint? - // Before the call it wasn't tainted, and it also shouldn't be tainted after the call. - - // CHECK-NEXT: PreCall prepares tainting arg index: -1 - // CHECK-NEXT: PreCall prepares tainting arg index: 0 - // CHECK-NEXT: PreCall prepares tainting arg index: 1 - // CHECK-NEXT: PreCall prepares tainting arg index: 2 - // - // CHECK-NEXT: PostCall actually wants to taint arg index: -1 - // CHECK-NEXT: PostCall actually wants to taint arg index: 0 - // CHECK-NEXT: PostCall actually wants to taint arg index: 1 - // CHECK-NEXT: PostCall actually wants to taint arg index: 2 -} -- 2.7.4