[quandary] gflag globals as source

Summary: Gflags is a popular library used to create command line arguments. Flags shouldn't flow directly to `exec` etc.

Reviewed By: jvillard, mbouaziz

Differential Revision: D5058393

fbshipit-source-id: ab062f8

infer/src/checkers/Trace.ml

@@ -189,9 +189,7 @@ module Make (Spec : Spec) = struct
           else acc in
         Sinks.fold report_one sinks acc0 in
       let report_sources source acc =
-        if Source.is_footprint source
+        report_source source t.sinks acc in
-        then acc
-        else report_source source t.sinks acc in
       Sources.fold report_sources t.sources []
 
   let pp_path cur_pname fmt (cur_passthroughs, sources_passthroughs, sinks_passthroughs) =

infer/src/quandary/ClangTrace.ml

@@ -182,10 +182,23 @@ include
       | (EnvironmentVariable | File), Allocation ->
           (* untrusted data flowing to memory allocation *)
           true
-      | Other, _
+      | _, (Allocation | Other | ShellExec) when Source.is_footprint source ->
-      | _, Other ->
+          (* is this var a command line flag created by the popular gflags library? *)
+          let is_gflag pvar =
+            String.is_substring ~substring:"FLAGS_" (Pvar.get_simplified_name pvar) in
+          begin
+            match Option.map ~f:AccessPath.extract (Source.get_footprint_access_path source) with
+            | Some ((Var.ProgramVar pvar, _), _) when Pvar.is_global pvar && is_gflag pvar ->
+                (* gflags globals come from the environment; treat them as sources *)
+                true
+            | _ ->
+                false
+          end
+      | Other, _ ->
           (* Other matches everything *)
           true
-      | _ ->
+      | _, Other ->
+          true
+      | Unknown, (Allocation | ShellExec) ->
           false
   end)

infer/src/quandary/JavaTrace.ml

@@ -339,21 +339,24 @@ include
     module Sink = JavaSink
 
     let should_report source sink =
-      match Source.kind source, Sink.kind sink with
+      if Source.is_footprint source
-      | PrivateData, Logging (* logging private data issue *)
+      then false
-      | Intent, StartComponent (* intent reuse issue *)
+      else
-      | Intent, CreateIntent (* intent configured with external values issue *)
+        match Source.kind source, Sink.kind sink with
-      | Intent, JavaScript (* external data flows into JS: remote code execution risk *)
+        | PrivateData, Logging (* logging private data issue *)
-      | PrivateData, JavaScript (* leaking private data into JS *)
+        | Intent, StartComponent (* intent reuse issue *)
-      | UserControlledURI, (CreateIntent | StartComponent)
+        | Intent, CreateIntent (* intent configured with external values issue *)
-      (* create intent/launch component from user-controlled URI *)
+        | Intent, JavaScript (* external data flows into JS: remote code execution risk *)
-      | UserControlledURI, CreateFile
+        | PrivateData, JavaScript (* leaking private data into JS *)
-      (* create file from user-controller URI; potential path-traversal vulnerability *)
+        | UserControlledURI, (CreateIntent | StartComponent)
-      | Clipboard, (StartComponent | CreateIntent | JavaScript | CreateFile) ->
+        (* create intent/launch component from user-controlled URI *)
-          (* do something sensitive with user-controlled data from the clipboard *)
+        | UserControlledURI, CreateFile
-          true
+        (* create file from user-controller URI; potential path-traversal vulnerability *)
-      | Other, _ | _, Other -> (* for testing purposes, Other matches everything *)
+        | Clipboard, (StartComponent | CreateIntent | JavaScript | CreateFile) ->
-          true
+            (* do something sensitive with user-controlled data from the clipboard *)
-      | _ ->
+            true
-          false
+        | Other, _ | _, Other -> (* for testing purposes, Other matches everything *)
+            true
+        | _ ->
+            false
   end)

infer/tests/codetoanalyze/cpp/quandary/execs.cpp

@@ -12,8 +12,12 @@
 
 extern int rand();
 
+// mocking gflags-generated field
+
 namespace execs {
 
+extern char* FLAGS_cli_string;
+
 int callAllSinks(const char* stringSource, char ** arrSource) {
   switch (rand()) {
     case 1:
@@ -88,4 +92,6 @@ void customGetEnvOk() {
   const char* source = execs::getenv("ENV_VAR");
   return execl(NULL, source);
 }
+
+void exec_flag_bad() { execl(FLAGS_cli_string, NULL); }
 }

infer/tests/codetoanalyze/cpp/quandary/issues.exp

@@ -32,6 +32,7 @@ codetoanalyze/cpp/quandary/execs.cpp, execs::callExecBad, 27, QUANDARY_TAINT_ERR
 codetoanalyze/cpp/quandary/execs.cpp, execs::callExecBad, 29, QUANDARY_TAINT_ERROR, [return from getenv,call to execve]
 codetoanalyze/cpp/quandary/execs.cpp, execs::callExecBad, 31, QUANDARY_TAINT_ERROR, [return from getenv,call to execve]
 codetoanalyze/cpp/quandary/execs.cpp, execs::callExecBad, 33, QUANDARY_TAINT_ERROR, [return from getenv,call to system]
+codetoanalyze/cpp/quandary/execs.cpp, execs::exec_flag_bad, 0, QUANDARY_TAINT_ERROR, [return from execs::exec_flag_bad,call to execl]
 codetoanalyze/cpp/quandary/files.cpp, files::read_file_call_exec_bad1, 5, QUANDARY_TAINT_ERROR, [return from std::basic_istream<char,std::char_traits<char>>_read,call to execle]
 codetoanalyze/cpp/quandary/files.cpp, files::read_file_call_exec_bad2, 5, QUANDARY_TAINT_ERROR, [return from std::basic_istream<char,std::char_traits<char>>_readsome,call to execle]
 codetoanalyze/cpp/quandary/files.cpp, files::read_file_call_exec_bad3, 5, QUANDARY_TAINT_ERROR, [return from std::basic_istream<char,std::char_traits<char>>_getline,call to execle]