Index: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp @@ -438,7 +438,7 @@ /// points to data, which should be tainted on return. REGISTER_MAP_WITH_PROGRAMSTATE(TaintArgsOnPostVisit, const LocationContext *, ImmutableSet) -ImmutableSet::Factory TaintArgsOnPostVisitFactory; +REGISTER_SET_FACTORY_WITH_PROGRAMSTATE(ArgIdxFactory, ArgIdxTy) void GenericTaintRuleParser::validateArgVector(const std::string &Option, const ArgVecTy &Args) const { @@ -559,7 +559,7 @@ {{"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 @@ if (TR::UntrustedEnv(C)) { // void setproctitle_init(int argc, char *argv[], char *envp[]) GlobalCRules.push_back( - {{{"setproctitle_init"}}, TR::Sink({{2}}, MsgCustomSink)}); + {{{"setproctitle_init"}}, TR::Sink({{1, 2}}, MsgCustomSink)}); GlobalCRules.push_back({{"getenv"}, TR::Source({{ReturnValueIndex}})}); } @@ -784,14 +784,17 @@ }; /// Propagate taint where it is necessary. - ImmutableSet Result = TaintArgsOnPostVisitFactory.getEmptySet(); + auto &F = State->getStateManager().get_context(); + ImmutableSet Result = F.getEmptySet(); + ForEachCallArg( - [this, WouldEscape, &Call, &Result](ArgIdxTy I, const Expr *E, SVal V) { + [this, WouldEscape, &Call, &Result, &F](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';); - Result = TaintArgsOnPostVisitFactory.add(Result, I); + Result = F.add(Result, I); } // TODO: We should traverse all reachable memory regions via the @@ -803,7 +806,7 @@ Call.dump(llvm::dbgs()); llvm::dbgs() << "> prepares tainting arg index: " << I << '\n'; }); - Result = TaintArgsOnPostVisitFactory.add(Result, I); + Result = F.add(Result, I); } }); @@ -900,8 +903,8 @@ return; ProgramStateRef State = C.getState(); - ImmutableSet Result = TaintArgsOnPostVisitFactory.add( - TaintArgsOnPostVisitFactory.getEmptySet(), ReturnValueIndex); + auto &F = State->getStateManager().get_context(); + ImmutableSet Result = F.add(F.getEmptySet(), ReturnValueIndex); State = State->set(C.getStackFrame(), Result); C.addTransition(State); } Index: clang/test/Analysis/taint-checker-callback-order-has-definition.c =================================================================== --- clang/test/Analysis/taint-checker-callback-order-has-definition.c +++ clang/test/Analysis/taint-checker-callback-order-has-definition.c @@ -25,11 +25,9 @@ (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 - + // // 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 } Index: clang/test/Analysis/taint-checker-callback-order-without-definition.c =================================================================== --- clang/test/Analysis/taint-checker-callback-order-without-definition.c +++ clang/test/Analysis/taint-checker-callback-order-without-definition.c @@ -19,16 +19,11 @@ (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 } Index: clang/test/Analysis/taint-generic.c =================================================================== --- clang/test/Analysis/taint-generic.c +++ clang/test/Analysis/taint-generic.c @@ -58,9 +58,11 @@ #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 @@ -404,3 +406,20 @@ 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}} +}