[analyzer] Introduce the assume-controlled-environment config option

[steakhal]

Summary

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.

[steakhal]

clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp

438

I'm checking this separately.

[martong]

LGTM!

clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def

331–337

I think we should explicitly mention getenv here.

[steakhal]

Mention getenv() in the config help.

[NoQ]

Aha, yeah, something like that.

clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def

 ANALYZER_OPTION(
     bool, ShouldConsiderSingleElementArraysAsFlexibleArrayMembers,
     "consider-single-element-arrays-as-flexible-array-members",
     "Consider single element arrays as flexible array member candidates. "
     "This will prevent the analyzer from assuming that a single element array "
     "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.
 //===----------------------------------------------------------------------===//
 
 ANALYZER_OPTION(unsigned, CTUImportThreshold, "ctu-import-threshold",
                 "The maximal amount of translation units that is considered "
                 "for import when inlining functions during CTU analysis. "
                 "Lowering this threshold can alleviate the memory burden of "

clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp

TaintPropagationRule Rule =
           // Source functions
           // TODO: Add support for vfscanf & family.
           .Case("fdopen", {{}, {ReturnValueIndex}})
           .Case("fopen", {{}, {ReturnValueIndex}})
           .Case("freopen", {{}, {ReturnValueIndex}})
           .Case("getch", {{}, {ReturnValueIndex}})
           .Case("getchar", {{}, {ReturnValueIndex}})
           .Case("getchar_unlocked", {{}, {ReturnValueIndex}})
-          .Case("getenv", {{}, {ReturnValueIndex}})
           .Case("gets", {{}, {0, ReturnValueIndex}})
           .Case("scanf", {{}, {}, VariadicType::Dst, 1})
           .Case("socket", {{},
                            {ReturnValueIndex},
                            VariadicType::None,
                            InvalidArgIndex,
                            &TaintPropagationRule::postSocket})
           .Case("wgetch", {{}, {ReturnValueIndex}})
TaintPropagationRule Rule =
           .Case("strchr", {{0}, {ReturnValueIndex}})
           .Case("strrchr", {{0}, {ReturnValueIndex}})
           .Case("tolower", {{0}, {ReturnValueIndex}})
           .Case("toupper", {{0}, {ReturnValueIndex}})
           .Default({});
 
   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.
   // This check is specialized but faster then calling isCLibraryFunction.
   const FunctionDecl *FDecl = FData.FDecl;
   unsigned BId = 0;
   if ((BId = FDecl->getMemoryFunctionKind())) {
     switch (BId) {

clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

enum CheckKind {
     CK_StdCLibraryFunctionsTesterChecker,
     CK_NumCheckKinds
   };
   DefaultBool ChecksEnabled[CK_NumCheckKinds];
   CheckerNameRef CheckNames[CK_NumCheckKinds];
 
   bool DisplayLoadedSummaries = false;
   bool ModelPOSIX = false;
+  bool ShouldAssumeControlledEnvironment = false;
 
 private:
   Optional<Summary> findFunctionSummary(const FunctionDecl *FD,
                                         CheckerContext &C) const;
   Optional<Summary> findFunctionSummary(const CallEvent &Call,
                                         CheckerContext &C) const;
 
   void initFunctionSummaries(CheckerContext &C) const;
void StdLibraryFunctionsChecker::initFunctionSummaries(
   //                  int delimiter, FILE *restrict stream);
   addToFunctionSummaryMap(
       "getdelim",
       Signature(ArgTypes{CharPtrPtrRestrictTy, SizePtrRestrictTy, IntTy,
                          FilePtrRestrictTy},
                 RetType{Ssize_tTy}),
       GetLineSummary);
 
+  {
+    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}),
-      Summary(NoEvalCall)
-          .Case({NotNull(Ret)})
-          .Case({NotNull(Ret)->negate()})
-          .ArgConstraint(NotNull(ArgNo(0))));
+        std::move(GetenvSummary));
+  }
 
   if (ModelPOSIX) {
 
     // long a64l(const char *str64);
     addToFunctionSummaryMap(
         "a64l", Signature(ArgTypes{ConstCharPtrTy}, RetType{LongTy}),
         Summary(NoEvalCall).ArgConstraint(NotNull(ArgNo(0))));
 
addToFunctionSummaryMap(
         Summary(EvalCallAsPure));
   }
 
   SummariesInitialized = true;
 }
 
 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(
     const CheckerManager &mgr) {
   return true;
 }
 
 #define REGISTER_CHECKER(name)                                                 \

clang/test/Analysis/analyzer-config.c

 // CHECK-NEXT: alpha.cplusplus.STLAlgorithmModeling:AggressiveStdFindModeling = false
 // CHECK-NEXT: alpha.osx.cocoa.DirectIvarAssignment:AnnotatedFunctions = false
 // CHECK-NEXT: alpha.security.MmapWriteExec:MmapProtExec = 0x04
 // CHECK-NEXT: alpha.security.MmapWriteExec:MmapProtRead = 0x01
 // CHECK-NEXT: alpha.security.taint.TaintPropagation:Config = ""
 // 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
 // CHECK-NEXT: c++-inlining = destructors
 // CHECK-NEXT: c++-shared_ptr-inlining = false
 // CHECK-NEXT: c++-stdlib-inlining = true
 // CHECK-NEXT: c++-temp-dtor-inlining = true
 // CHECK-NEXT: c++-template-inlining = true

clang/test/Analysis/assume-controlled-environment.c

+// 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}}
+}