Author: Balazs Benics Date: 2021-10-13T10:50:26+02:00 New Revision: edde4efc66df2257f0b2351d5f98b4fbb2ced620
URL: https://github.com/llvm/llvm-project/commit/edde4efc66df2257f0b2351d5f98b4fbb2ced620 DIFF: https://github.com/llvm/llvm-project/commit/edde4efc66df2257f0b2351d5f98b4fbb2ced620.diff LOG: [analyzer] Introduce the assume-controlled-environment config option If the `assume-controlled-environment` is `true`, we should expect `getenv()` to succeed, and the result should not be considered tainted. By default, the option will be `false`. Reviewed By: NoQ, martong Differential Revision: https://reviews.llvm.org/D111296 Added: clang/test/Analysis/assume-controlled-environment.c Modified: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp clang/test/Analysis/analyzer-config.c Removed: ################################################################################ diff --git a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def index 16ca7a8456e39..aab8e1284bf64 100644 --- a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def +++ b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def @@ -328,6 +328,14 @@ ANALYZER_OPTION( "holds a single element.", false) +ANALYZER_OPTION( + bool, ShouldAssumeControlledEnvironment, "assume-controlled-environment", + "Whether the analyzed application runs in a controlled environment. " + "We will assume that environment variables exist in queries and they hold " + "no malicious data. For instance, if this option is enabled, 'getenv()' " + "might be modeled by the analyzer to never return NULL.", + false) + //===----------------------------------------------------------------------===// // Unsigned analyzer options. //===----------------------------------------------------------------------===// diff --git a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp index 42c777eb2c521..3fc046015f151 100644 --- a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp @@ -435,7 +435,6 @@ GenericTaintChecker::TaintPropagationRule::getTaintPropagationRule( .Case("getch", {{}, {ReturnValueIndex}}) .Case("getchar", {{}, {ReturnValueIndex}}) .Case("getchar_unlocked", {{}, {ReturnValueIndex}}) - .Case("getenv", {{}, {ReturnValueIndex}}) .Case("gets", {{}, {0, ReturnValueIndex}}) .Case("scanf", {{}, {}, VariadicType::Dst, 1}) .Case("socket", {{}, @@ -468,6 +467,16 @@ GenericTaintChecker::TaintPropagationRule::getTaintPropagationRule( if (!Rule.isNull()) return Rule; + + // `getenv` returns taint only in untrusted environments. + if (FData.FullName == "getenv") { + if (C.getAnalysisManager() + .getAnalyzerOptions() + .ShouldAssumeControlledEnvironment) + return {}; + return {{}, {ReturnValueIndex}}; + } + assert(FData.FDecl); // Check if it's one of the memory setting/copying functions. diff --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp index 74adc5882bfbf..e8b963a535d8b 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp @@ -568,6 +568,7 @@ class StdLibraryFunctionsChecker bool DisplayLoadedSummaries = false; bool ModelPOSIX = false; + bool ShouldAssumeControlledEnvironment = false; private: Optional<Summary> findFunctionSummary(const FunctionDecl *FD, @@ -1433,13 +1434,19 @@ void StdLibraryFunctionsChecker::initFunctionSummaries( RetType{Ssize_tTy}), GetLineSummary); - // char *getenv(const char *name); - addToFunctionSummaryMap( - "getenv", Signature(ArgTypes{ConstCharPtrTy}, RetType{CharPtrTy}), - Summary(NoEvalCall) - .Case({NotNull(Ret)}) - .Case({NotNull(Ret)->negate()}) - .ArgConstraint(NotNull(ArgNo(0)))); + { + Summary GetenvSummary = Summary(NoEvalCall) + .ArgConstraint(NotNull(ArgNo(0))) + .Case({NotNull(Ret)}); + // In untrusted environments the envvar might not exist. + if (!ShouldAssumeControlledEnvironment) + GetenvSummary.Case({NotNull(Ret)->negate()}); + + // char *getenv(const char *name); + addToFunctionSummaryMap( + "getenv", Signature(ArgTypes{ConstCharPtrTy}, RetType{CharPtrTy}), + std::move(GetenvSummary)); + } if (ModelPOSIX) { @@ -2653,11 +2660,12 @@ void StdLibraryFunctionsChecker::initFunctionSummaries( void ento::registerStdCLibraryFunctionsChecker(CheckerManager &mgr) { auto *Checker = mgr.registerChecker<StdLibraryFunctionsChecker>(); + const AnalyzerOptions &Opts = mgr.getAnalyzerOptions(); Checker->DisplayLoadedSummaries = - mgr.getAnalyzerOptions().getCheckerBooleanOption( - Checker, "DisplayLoadedSummaries"); - Checker->ModelPOSIX = - mgr.getAnalyzerOptions().getCheckerBooleanOption(Checker, "ModelPOSIX"); + Opts.getCheckerBooleanOption(Checker, "DisplayLoadedSummaries"); + Checker->ModelPOSIX = Opts.getCheckerBooleanOption(Checker, "ModelPOSIX"); + Checker->ShouldAssumeControlledEnvironment = + Opts.ShouldAssumeControlledEnvironment; } bool ento::shouldRegisterStdCLibraryFunctionsChecker( diff --git a/clang/test/Analysis/analyzer-config.c b/clang/test/Analysis/analyzer-config.c index 4cca1cb553b62..429229f6b6ca4 100644 --- a/clang/test/Analysis/analyzer-config.c +++ b/clang/test/Analysis/analyzer-config.c @@ -15,6 +15,7 @@ // CHECK-NEXT: apiModeling.StdCLibraryFunctions:DisplayLoadedSummaries = false // CHECK-NEXT: apiModeling.StdCLibraryFunctions:ModelPOSIX = false // CHECK-NEXT: apply-fixits = false +// CHECK-NEXT: assume-controlled-environment = false // CHECK-NEXT: avoid-suppressing-null-argument-paths = false // CHECK-NEXT: c++-allocator-inlining = true // CHECK-NEXT: c++-container-inlining = false diff --git a/clang/test/Analysis/assume-controlled-environment.c b/clang/test/Analysis/assume-controlled-environment.c new file mode 100644 index 0000000000000..749b8198f6fb0 --- /dev/null +++ b/clang/test/Analysis/assume-controlled-environment.c @@ -0,0 +1,22 @@ +// RUN: %clang_analyze_cc1 -verify=untrusted-env %s \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-checker=alpha.security.taint \ +// RUN: -analyzer-checker=debug.TaintTest + +// RUN: %clang_analyze_cc1 -verify %s -DEXPECT_NO_WARNINGS \ +// RUN: -analyzer-config assume-controlled-environment=true \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-checker=alpha.security.taint \ +// RUN: -analyzer-checker=debug.TaintTest + + +#ifdef EXPECT_NO_WARNINGS +// expected-no-diagnostics +#endif + +char *getenv(const char *name); + +void foo() { + char *p = getenv("FOO"); // untrusted-env-warning {{tainted}} + (void)p; // untrusted-env-warning {{tainted}} +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
