Author: Balazs Benics Date: 2022-02-14T18:45:46+01:00 New Revision: 2acead35c1289d2b3593a992b0639ca6427e481f
URL: https://github.com/llvm/llvm-project/commit/2acead35c1289d2b3593a992b0639ca6427e481f DIFF: https://github.com/llvm/llvm-project/commit/2acead35c1289d2b3593a992b0639ca6427e481f.diff LOG: Revert "[analyzer] Fix taint rule of fgets and setproctitle_init" This reverts commit bf5963bf19670ea58facdf57492e147c13bb650f. I'm reverting this since the head of the patch stack caused a build breakage. https://lab.llvm.org/buildbot/#/builders/91/builds/3818 Added: Modified: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp clang/test/Analysis/taint-checker-callback-order-has-definition.c clang/test/Analysis/taint-checker-callback-order-without-definition.c clang/test/Analysis/taint-generic.c Removed: ################################################################################ diff --git a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp index d15a4659a96e..66143f78932c 100644 --- a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp @@ -559,7 +559,7 @@ void GenericTaintChecker::initTaintRules(CheckerContext &C) const { {{"atoll"}, TR::Prop({{0}}, {{ReturnValueIndex}})}, {{"fgetc"}, TR::Prop({{0}}, {{ReturnValueIndex}})}, {{"fgetln"}, TR::Prop({{0}}, {{ReturnValueIndex}})}, - {{"fgets"}, TR::Prop({{2}}, {{0, ReturnValueIndex}})}, + {{"fgets"}, TR::Prop({{2}}, {{0}, ReturnValueIndex})}, {{"fscanf"}, TR::Prop({{0}}, {{}, 2})}, {{"sscanf"}, TR::Prop({{0}}, {{}, 2})}, {{"getc"}, TR::Prop({{0}}, {{ReturnValueIndex}})}, @@ -632,7 +632,7 @@ void GenericTaintChecker::initTaintRules(CheckerContext &C) const { if (TR::UntrustedEnv(C)) { // void setproctitle_init(int argc, char *argv[], char *envp[]) GlobalCRules.push_back( - {{{"setproctitle_init"}}, TR::Sink({{1, 2}}, MsgCustomSink)}); + {{{"setproctitle_init"}}, TR::Sink({{2}}, MsgCustomSink)}); GlobalCRules.push_back({{"getenv"}, TR::Source({{ReturnValueIndex}})}); } diff --git a/clang/test/Analysis/taint-checker-callback-order-has-definition.c b/clang/test/Analysis/taint-checker-callback-order-has-definition.c index cdf55e3aef41..a070077004fa 100644 --- a/clang/test/Analysis/taint-checker-callback-order-has-definition.c +++ b/clang/test/Analysis/taint-checker-callback-order-has-definition.c @@ -25,9 +25,11 @@ void top(const char *fname, char *buf) { (void)fgets(buf, 42, fp); // Trigger taint propagation. // CHECK-NEXT: PreCall<fgets(buf, 42, fp)> prepares tainting arg index: -1 // CHECK-NEXT: PreCall<fgets(buf, 42, fp)> prepares tainting arg index: 0 + // CHECK-NEXT: PreCall<fgets(buf, 42, fp)> prepares tainting arg index: 1 // CHECK-NEXT: PreCall<fgets(buf, 42, fp)> prepares tainting arg index: 2 - // + // CHECK-NEXT: PostCall<fgets(buf, 42, fp)> actually wants to taint arg index: -1 // CHECK-NEXT: PostCall<fgets(buf, 42, fp)> actually wants to taint arg index: 0 + // CHECK-NEXT: PostCall<fgets(buf, 42, fp)> actually wants to taint arg index: 1 // CHECK-NEXT: PostCall<fgets(buf, 42, fp)> actually wants to taint arg index: 2 } diff --git a/clang/test/Analysis/taint-checker-callback-order-without-definition.c b/clang/test/Analysis/taint-checker-callback-order-without-definition.c index f2e070739e97..962e8b259298 100644 --- a/clang/test/Analysis/taint-checker-callback-order-without-definition.c +++ b/clang/test/Analysis/taint-checker-callback-order-without-definition.c @@ -19,11 +19,16 @@ void top(const char *fname, char *buf) { (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<fgets(buf, 42, fp)> prepares tainting arg index: -1 // CHECK-NEXT: PreCall<fgets(buf, 42, fp)> prepares tainting arg index: 0 + // CHECK-NEXT: PreCall<fgets(buf, 42, fp)> prepares tainting arg index: 1 // CHECK-NEXT: PreCall<fgets(buf, 42, fp)> prepares tainting arg index: 2 // // CHECK-NEXT: PostCall<fgets(buf, 42, fp)> actually wants to taint arg index: -1 // CHECK-NEXT: PostCall<fgets(buf, 42, fp)> actually wants to taint arg index: 0 + // CHECK-NEXT: PostCall<fgets(buf, 42, fp)> actually wants to taint arg index: 1 // CHECK-NEXT: PostCall<fgets(buf, 42, fp)> actually wants to taint arg index: 2 } diff --git a/clang/test/Analysis/taint-generic.c b/clang/test/Analysis/taint-generic.c index 0612e1b9f98b..6979c0667764 100644 --- a/clang/test/Analysis/taint-generic.c +++ b/clang/test/Analysis/taint-generic.c @@ -58,11 +58,9 @@ extern FILE *stdin; #define bool _Bool -char *getenv(const char *name); int fscanf(FILE *restrict stream, const char *restrict format, ...); int sprintf(char *str, const char *format, ...); void setproctitle(const char *fmt, ...); -void setproctitle_init(int argc, char *argv[], char *envp[]); typedef __typeof(sizeof(int)) size_t; // Define string functions. Use builtin for some of them. They all default to @@ -406,20 +404,3 @@ void testConfigurationSinks(void) { void testUnknownFunction(void (*foo)(void)) { foo(); // no-crash } - -void testProctitleFalseNegative() { - char flag[80]; - fscanf(stdin, "%79s", flag); - char *argv[] = {"myapp", flag}; - // FIXME: We should have a warning below: Untrusted data passed to sink. - setproctitle_init(1, argv, 0); -} - -void testProctitle2(char *real_argv[]) { - char *app = getenv("APP_NAME"); - if (!app) - return; - char *argv[] = {app, "--foobar"}; - 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}} -} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits