[clang][analyzer] Add taintedness to argv (llvm#178054)

If the execution environment is untrusted, we assume that the argv, argc
and envp parameters of the main function are attacker controlled values
and set them as taint analysis sources.

Author: dkrupp

clang/docs/analyzer/checkers.rst

@@ -1482,6 +1482,13 @@ For a more detailed description of configuration options, please see the
    advanced users can fully customize their taint configuration model.
    Default: ``true``.
 
+* If the analyzer option ``assume-controlled-environment`` is set to ``false``,
+  it is assumed that the command line arguments and the environment
+  variables of the program are attacker controlled.
+  In particular, the ``argv``, ``argc`` and ``envp`` arguments of the
+  ``main`` function and the return value of the ``getenv()``
+  function are assumed to hold tainted values.
+
 **Related Guidelines**
 
 * `CWE Data Neutralization Issues
@@ -3788,7 +3795,7 @@ Check that ``[[clang::annotate_type("webkit.nodelete")]]`` annotation does not a
  Foo [[clang::annotate_type("webkit.nodelete")]] trivialFunction(RefCountable* obj) {
    return obj->anotherTrivialFunction();
  };
- 
+
 ``[[clang::annotate_type("webkit.nodelete")]]`` annotation makes the function ignored for the purpose of other WebKit smart pointer checkers.
 For example, ``alpha.webkit.UncountedCallArgsChecker`` will ignore a function call with this annotation.
 

clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp

@@ -378,10 +378,12 @@ struct GenericTaintRuleParser {
   CheckerManager &Mgr;
 };
 
-class GenericTaintChecker : public Checker<check::PreCall, check::PostCall> {
+class GenericTaintChecker
+    : public Checker<check::PreCall, check::PostCall, check::BeginFunction> {
 public:
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
   void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
+  void checkBeginFunction(CheckerContext &C) const;
 
   void printState(raw_ostream &Out, ProgramStateRef State, const char *NL,
                   const char *Sep) const override;
@@ -829,8 +831,94 @@ void GenericTaintChecker::initTaintRules(CheckerContext &C) const {
                             std::make_move_iterator(Rules.end()));
 }
 
+bool isPointerToCharArray(const QualType &QT) {
+  if (!QT->isPointerType())
+    return false;
+  QualType PointeeType = QT->getPointeeType();
+  return PointeeType->isPointerType() &&
+         PointeeType->getPointeeType()->isCharType();
+}
+
+// The incoming parameters of the main function get tainted
+// if the program called in an untrusted environment.
+void GenericTaintChecker::checkBeginFunction(CheckerContext &C) const {
+  if (!C.inTopFrame() || C.getAnalysisManager()
+                             .getAnalyzerOptions()
+                             .ShouldAssumeControlledEnvironment)
+    return;
+
+  const auto *FD = dyn_cast<FunctionDecl>(C.getLocationContext()->getDecl());
+  if (!FD || !FD->isMain() || FD->param_size() < 2)
+    return;
+
+  if (!FD->parameters()[0]->getType()->isIntegerType())
+    return;
+
+  if (!isPointerToCharArray(FD->parameters()[1]->getType()))
+    return;
+  ProgramStateRef State = C.getState();
+
+  const MemRegion *ArgcReg =
+      State->getRegion(FD->parameters()[0], C.getLocationContext());
+  SVal ArgcSVal = State->getSVal(ArgcReg);
+  State = addTaint(State, ArgcSVal);
+  StringRef ArgcName = FD->parameters()[0]->getName();
+  if (auto N = ArgcSVal.getAs<NonLoc>()) {
+    ConstraintManager &CM = C.getConstraintManager();
+    // The upper bound is the ARG_MAX on an arbitrary Linux
+    // to model that is is typically smaller than INT_MAX.
+    State = CM.assumeInclusiveRange(State, *N, llvm::APSInt::getUnsigned(1),
+                                    llvm::APSInt::getUnsigned(2097152), true);
+  }
+
+  const MemRegion *ArgvReg =
+      State->getRegion(FD->parameters()[1], C.getLocationContext());
+  SVal ArgvSVal = State->getSVal(ArgvReg);
+  State = addTaint(State, ArgvSVal);
+  StringRef ArgvName = FD->parameters()[1]->getName();
+
+  bool HaveEnvp = FD->param_size() > 2;
+  SVal EnvpSVal;
+  StringRef EnvpName;
+  if (HaveEnvp && !isPointerToCharArray(FD->parameters()[2]->getType()))
+    return;
+  if (HaveEnvp) {
+    const MemRegion *EnvPReg =
+        State->getRegion(FD->parameters()[2], C.getLocationContext());
+    EnvpSVal = State->getSVal(EnvPReg);
+    EnvpName = FD->parameters()[2]->getName();
+    State = addTaint(State, EnvpSVal);
+  }
+
+  const NoteTag *OriginatingTag =
+      C.getNoteTag([ArgvSVal, ArgcSVal, ArgcName, ArgvName, EnvpSVal,
+                    EnvpName](PathSensitiveBugReport &BR) -> std::string {
+        if ((!BR.isInteresting(ArgcSVal) && !BR.isInteresting(ArgvSVal) &&
+             !BR.isInteresting(EnvpSVal)))
+          return "";
+        if (BR.getBugType().getCategory() != categories::TaintedData)
+          return "";
+        std::string Message = "";
+        if (BR.isInteresting(ArgvSVal))
+          Message += "'" + ArgvName.str() + "'";
+        if (BR.isInteresting(ArgcSVal)) {
+          if (Message.size() > 0)
+            Message += ", ";
+          Message += "'" + ArgcName.str() + "'";
+        }
+        if (BR.isInteresting(EnvpSVal)) {
+          if (Message.size() > 0)
+            Message += ", ";
+          Message += "'" + EnvpName.str() + "'";
+        }
+        return "Taint originated in " + Message;
+      });
+  C.addTransition(State, OriginatingTag);
+}
+
 void GenericTaintChecker::checkPreCall(const CallEvent &Call,
                                        CheckerContext &C) const {
+
   initTaintRules(C);
 
   // FIXME: this should be much simpler.

clang/test/Analysis/taint-generic.c

@@ -116,6 +116,7 @@ char *stpcpy(char *restrict s1, const char *restrict s2);
 char *strncpy( char * destination, const char * source, size_t num );
 char *strndup(const char *s, size_t n);
 char *strncat(char *restrict s1, const char *restrict s2, size_t n);
+char *strcat( char *dest, const char *src );
 
 void *malloc(size_t);
 void *calloc(size_t nmemb, size_t size);
@@ -1396,3 +1397,13 @@ void testAcceptPropagates() {
   int acceptSocket = accept(listenSocket, 0, 0);
   clang_analyzer_isTainted_int(acceptSocket); // expected-warning {{YES}}
 }
+
+int main(int argc, char * argv[]) {
+   if (argc < 2)
+     return 1;
+   char cmd[2048] = "/bin/cat ";
+   clang_analyzer_isTainted_char(*argv[1]); // expected-warning{{YES}}
+   strncat(cmd, argv[1], sizeof(cmd) - strlen(cmd)-1);
+   system(cmd);// expected-warning {{Untrusted data is passed to a system call}}
+   return 0;
+ }

clang/test/Analysis/taint-main-parameters/argc.c

@@ -0,0 +1,18 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=optin.taint,core,security.ArrayBound \
+// RUN:   -analyzer-config assume-controlled-environment=false -analyzer-output=text -verify %s
+
+// This file is for testing enhanced diagnostics produced by the
+// GenericTaintChecker
+
+// In an untrusted environment the argc is also tainted.
+int main(int argc, char *argv[],  char *envp[]) { // expected-note {{Taint originated in 'argc'}}
+  char cmd[2048] = "/bin/cat ";
+  char filename[1024];
+  int foo = 100 / argc; // no-warning argc >= 1
+  if (!envp[0]) // expected-note {{Assuming the condition is false}}
+                // expected-note@-1 {{Taking false branch}}
+     return 1;
+   int v[5]={1,2,3,4,5};
+   return v[argc];// expected-warning {{Potential out of bound access to 'v' with tainted index}}
+                  // expected-note@-1 {{Access of 'v' with a tainted index that may be too large}}
+ }

clang/test/Analysis/taint-main-parameters/envp.c

@@ -0,0 +1,26 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=optin.taint,core,security.ArrayBound \
+// RUN:   -analyzer-config assume-controlled-environment=false -analyzer-output=text -verify %s
+
+// This file is for testing enhanced diagnostics produced by the
+// GenericTaintChecker
+
+typedef __typeof(sizeof(int)) size_t;
+int system(const char *command);
+size_t strlen(const char *str);
+char *strncat(char *destination, const char *source, size_t num);
+char *strncpy(char *destination, const char *source, size_t num);
+
+// In an untrusted environment the the environment variables
+// coming through the envp are also tainted.
+int main(int argc, char *argv[],  char *envp[]) { // expected-note {{Taint originated in 'envp'}}
+  char cmd[2048] = "/bin/cat ";
+  char filename[1024];
+  if (!envp[0]) // expected-note {{Assuming the condition is false}}
+                // expected-note@-1 {{Taking false branch}}
+    return 1;
+  strncpy(filename, envp[0], sizeof(filename) - 1); // expected-note {{Taint propagated to the 1st argument}}
+  strncat(cmd, filename, sizeof(cmd) - strlen(cmd) - 1); // expected-note {{Taint propagated to the 1st argument}}
+  system(cmd); // expected-warning {{Untrusted data is passed to a system call}}
+               // expected-note@-1 {{Untrusted data is passed to a system call}}
+  return 0;
+}

clang/test/Analysis/taint-main-parameters/envp2.c

@@ -0,0 +1,28 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=optin.taint,core,security.ArrayBound \
+// RUN:   -analyzer-config assume-controlled-environment=false -analyzer-output=text -verify %s
+
+// This file is for testing enhanced diagnostics produced by the
+// GenericTaintChecker
+
+typedef __typeof(sizeof(int)) size_t;
+int system(const char *command);
+size_t strlen(const char *str);
+char *strncat(char *destination, const char *source, size_t num);
+char *strncpy(char *destination, const char *source, size_t num);
+
+// In an untrusted environment the the environment variables
+// coming through the envp are also tainted.
+// This differs from envp.c because main parameters are declared
+// with alternative pointer to pointer declaration.
+int main(int argc, char **argv,  char **envp) { // expected-note {{Taint originated in 'envp'}}
+  char cmd[2048] = "/bin/cat ";
+  char filename[1024];
+  if (!envp[0]) // expected-note {{Assuming the condition is false}}
+                // expected-note@-1 {{Taking false branch}}
+    return 1;
+  strncpy(filename, envp[0], sizeof(filename) - 1); // expected-note {{Taint propagated to the 1st argument}}
+  strncat(cmd, filename, sizeof(cmd) - strlen(cmd) - 1); // expected-note {{Taint propagated to the 1st argument}}
+  system(cmd); // expected-warning {{Untrusted data is passed to a system call}}
+               // expected-note@-1 {{Untrusted data is passed to a system call}}
+  return 0;
+}

clang/test/Analysis/taint-main-parameters/invalid.c

@@ -0,0 +1,20 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=optin.taint,core,security.ArrayBound -analyzer-config \
+// RUN:   assume-controlled-environment=false -analyzer-output=text -verify %s
+
+typedef __typeof(sizeof(int)) size_t;
+int system(const char *command);
+size_t strlen(const char *str);
+char *strncat(char *destination, const char *source, size_t num);
+
+// When main is declared with incorrect signature,
+// the analyzer won't crash and we get a compilation error.
+
+int main(char* argv[], int argc) {
+  //expected-error@-1 {{first parameter of 'main' (argument count) must be of type 'int'}}
+  //expected-error@-2 {{second parameter of 'main' (argument array) must be of type 'char **'}}
+  char cmd[2048] = "/bin/cat ";
+  char filename[1024];
+  strncat(cmd, filename, sizeof(cmd) - strlen(cmd) - 1);
+  system(cmd);
+  return 0;
+}

clang/test/Analysis/taint-main-parameters/main_void.c

@@ -0,0 +1,19 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=optin.taint,core,security.ArrayBound -analyzer-config \
+// RUN:   assume-controlled-environment=false -analyzer-output=text -verify %s
+// expected-no-diagnostics
+
+// This file is for testing enhanced
+// diagnostics produced by the GenericTaintChecker
+
+typedef __typeof(sizeof(int)) size_t;
+int system(const char *command);
+size_t strlen(const char *str);
+char *strncat(char *destination, const char *source, size_t num);
+
+int main(void) {
+  char cmd[2048] = "/bin/cat ";
+  char filename[1024];
+  strncat(cmd, filename, sizeof(cmd) - strlen(cmd) - 1);
+  system(cmd);
+  return 0;
+}

clang/test/Analysis/taint-main-parameters/simple.c

@@ -0,0 +1,25 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=optin.taint,core,security.ArrayBound \
+// RUN: -analyzer-config assume-controlled-environment=false -analyzer-output=text -verify %s
+
+// This file is for testing enhanced diagnostics produced by the GenericTaintChecker
+
+typedef __typeof(sizeof(int)) size_t;
+int system(const char *command);
+size_t strlen(const char *str);
+char *strncat(char *destination, const char *source, size_t num);
+char *strncpy(char *destination, const char *source, size_t num);
+
+// In an untrusted environment the cmd line arguments
+// are assumed to be tainted.
+int main(int argc, char *argv[]) { // expected-note {{Taint originated in 'argv'}}
+  if (argc < 2)          // expected-note {{'argc' is >= 2}}
+                         // expected-note@-1 {{Taking false branch}}
+    return 1;
+  char cmd[2048] = "/bin/cat ";
+  char filename[1024];
+  strncpy(filename, argv[1], sizeof(filename) - 1); // expected-note {{Taint propagated to the 1st argument}}
+  strncat(cmd, filename, sizeof(cmd) - strlen(cmd) - 1); // expected-note {{Taint propagated to the 1st argument}}
+  system(cmd); // expected-warning {{Untrusted data is passed to a system call}}
+               // expected-note@-1 {{Untrusted data is passed to a system call}}
+  return 0;
+}

clang/test/Analysis/taint-main-parameters/simple.cpp

@@ -0,0 +1,43 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=optin.taint,core,security.ArrayBound -analyzer-config \
+// RUN:   assume-controlled-environment=false -analyzer-output=text -verify %s
+
+// This file is for testing enhanced diagnostics produced by the
+// GenericTaintChecker
+
+typedef __typeof(sizeof(int)) size_t;
+int system(const char *command);
+size_t strlen(const char *str);
+char *strncat(char *destination, const char *source, size_t num);
+char *strncpy(char *destination, const char *source, size_t num);
+
+// In an untrusted environment the cmd line arguments
+// are assumed to be tainted.
+int main(int argc, char *argv[]) { // expected-note {{Taint originated in 'argv'}}
+  if (argc < 2)          // expected-note {{'argc' is >= 2}}
+                         // expected-note@-1 {{Taking false branch}}
+    return 1;
+  char cmd[2048] = "/bin/cat ";
+  char filename[1024];
+  strncpy(filename, argv[1], sizeof(filename) - 1); // expected-note {{Taint propagated to the 1st argument}}
+  strncat(cmd, filename, sizeof(cmd) - strlen(cmd) - 1); // expected-note {{Taint propagated to the 1st argument}}
+  system(cmd); // expected-warning {{Untrusted data is passed to a system call}}
+               // expected-note@-1 {{Untrusted data is passed to a system call}}
+  return 0;
+}
+
+// A function declared inside a class or namespace may be named "main" but it
+// cannot be _the_ `main` function that is executed at startup. Validate that
+// in a case like this the arguments are not marked as tainted and no warning
+// is produced.
+class MyClass {
+  int main(int argc, char *argv[]) {
+    if (argc < 2)
+      return 1;
+    char cmd[2048] = "/bin/cat ";
+    char filename[1024];
+    strncpy(filename, argv[1], sizeof(filename) - 1);
+    strncat(cmd, filename, sizeof(cmd) - strlen(cmd) - 1);
+    system(cmd);
+    return 0;
+  }
+};