[quandary] propagate taint across unary/binary operators

Reviewed By: jeremydubreil

Differential Revision: D6318463

fbshipit-source-id: c73f003

infer/src/quandary/TaintAnalysis.ml

@@ -72,10 +72,21 @@ module Make (TaintSpecification : TaintSpec.S) = struct
 
 
     (* get the node associated with [exp] in [access_tree] *)
-    let hil_exp_get_node ?(abstracted= false) (exp: HilExp.t) access_tree proc_data =
+    let rec hil_exp_get_node ?(abstracted= false) (exp: HilExp.t) access_tree proc_data =
       match exp with
       | AccessPath access_path ->
           exp_get_node_ ~abstracted access_path access_tree proc_data
+      | Cast (_, e) | Exception e | UnaryOperator (_, e, _) ->
+          hil_exp_get_node ~abstracted e access_tree proc_data
+      | BinaryOperator (_, e1, e2) -> (
+        match
+          ( hil_exp_get_node ~abstracted e1 access_tree proc_data
+          , hil_exp_get_node ~abstracted e2 access_tree proc_data )
+        with
+        | Some node1, Some node2 ->
+            Some (TaintDomain.node_join node1 node2)
+        | node_opt, None | None, node_opt ->
+            node_opt )
       | _ ->
           None
 
@@ -267,39 +278,40 @@ module Make (TaintSpecification : TaintSpec.S) = struct
       (* add [sink] to the trace associated with the [formal_index]th actual *)
       let add_sink_to_actual sink_index access_tree_acc =
         match List.nth actuals sink_index with
-        | Some HilExp.AccessPath actual_ap_raw
-          -> (
-            let actual_ap = AccessPath.Abs.Abstracted actual_ap_raw in
-            match access_path_get_node actual_ap access_tree_acc proc_data with
-            | Some (actual_trace, _) ->
-                let sink' =
-                  let indexes = TraceDomain.get_footprint_indexes actual_trace in
-                  TraceDomain.Sink.make ~indexes (TraceDomain.Sink.kind sink) callee_site
-                in
-                let actual_trace' = TraceDomain.add_sink sink' actual_trace in
-                report_trace actual_trace' callee_site proc_data ;
-                if TraceDomain.Sources.Footprint.is_empty
-                     (TraceDomain.sources actual_trace').footprint
-                then
+        | Some exp -> (
+          match hil_exp_get_node ~abstracted:true exp access_tree_acc proc_data with
+          | Some (actual_trace, _)
+            -> (
+              let sink' =
+                let indexes = TraceDomain.get_footprint_indexes actual_trace in
+                TraceDomain.Sink.make ~indexes (TraceDomain.Sink.kind sink) callee_site
+              in
+              let actual_trace' = TraceDomain.add_sink sink' actual_trace in
+              report_trace actual_trace' callee_site proc_data ;
+              match exp with
+              | HilExp.AccessPath actual_ap_raw
+                when not
+                       (TraceDomain.Sources.Footprint.is_empty
+                          (TraceDomain.sources actual_trace').footprint) ->
+                  let actual_ap = AccessPath.Abs.Abstracted actual_ap_raw in
+                  TaintDomain.add_trace actual_ap actual_trace' access_tree_acc
+              | _ ->
                   (* no more sources can flow into this sink; no sense in keeping track of it *)
-                  access_tree_acc
-                else TaintDomain.add_trace actual_ap actual_trace' access_tree_acc
-            | _ ->
-                access_tree_acc )
+                  access_tree_acc )
+          | _ ->
+              access_tree_acc )
         | None ->
             L.internal_error
               "Taint is supposed to flow into sink %a at index %d, but the index is out of bounds"
               CallSite.pp callee_site sink_index ;
             access_tree_acc
-        | _ ->
-            access_tree_acc
       in
       IntSet.fold add_sink_to_actual (TraceDomain.Sink.indexes sink) access_tree
 
 
     let apply_summary ret_opt (actuals: HilExp.t list) summary caller_access_tree
         (proc_data: extras ProcData.t) callee_site =
-      let get_caller_ap formal_ap =
+      let get_caller_ap_node_opt formal_ap access_tree =
         let apply_return ret_ap =
           match ret_opt with
           | Some base_var ->
@@ -310,11 +322,6 @@ module Make (TaintSpecification : TaintSpec.S) = struct
                 AccessPath.Abs.pp ret_ap Typ.Procname.pp (CallSite.pname callee_site) ;
               None
         in
-        let get_actual_ap formal_index =
-          Option.value_map
-            ~f:(function HilExp.AccessPath access_path -> Some access_path | _ -> None)
-            ~default:None (List.nth actuals formal_index)
-        in
         let project ~formal_ap ~actual_ap =
           let projected_ap =
             AccessPath.append actual_ap (snd (AccessPath.Abs.extract formal_ap))
@@ -325,36 +332,38 @@ module Make (TaintSpecification : TaintSpec.S) = struct
         let base_var, _ = fst (AccessPath.Abs.extract formal_ap) in
         match base_var with
         | Var.ProgramVar pvar ->
-            if Pvar.is_return pvar then apply_return formal_ap else Some formal_ap
+            let projected_ap_opt =
+              if Pvar.is_return pvar then apply_return formal_ap else Some formal_ap
+            in
+            let caller_node_opt =
+              match projected_ap_opt with
+              | Some projected_ap ->
+                  access_path_get_node projected_ap access_tree proc_data
+              | None ->
+                  None
+            in
+            (projected_ap_opt, Option.value ~default:TaintDomain.empty_node caller_node_opt)
         | Var.LogicalVar id when Ident.is_footprint id -> (
-          match
-            (* summaries store the index of the formal parameter in the ident stamp *)
-            get_actual_ap (Ident.get_stamp id)
-          with
-          | Some actual_ap ->
+          match List.nth actuals (Ident.get_stamp id) with
+          | Some HilExp.AccessPath actual_ap ->
               let projected_ap = project ~formal_ap ~actual_ap in
-              Some projected_ap
-          | None ->
-              None )
+              let caller_node_opt = access_path_get_node projected_ap access_tree proc_data in
+              (Some projected_ap, Option.value ~default:TaintDomain.empty_node caller_node_opt)
+          | Some exp ->
+              let caller_node_opt = hil_exp_get_node exp access_tree proc_data in
+              (None, Option.value ~default:TaintDomain.empty_node caller_node_opt)
+          | _ ->
+              (None, TaintDomain.empty_node) )
         | _ ->
-            None
-      in
-      let get_caller_ap_node_opt ap access_tree =
-        let get_caller_node caller_ap =
-          let caller_node_opt = access_path_get_node caller_ap access_tree proc_data in
-          let caller_node = Option.value ~default:TaintDomain.empty_node caller_node_opt in
-          (caller_ap, caller_node)
-        in
-        Option.map (get_caller_ap ap) ~f:get_caller_node
+            (None, TaintDomain.empty_node)
       in
       let replace_footprint_sources callee_trace caller_trace access_tree =
         let replace_footprint_source acc footprint_access_path (is_mem, _) =
           if is_mem then
-            match get_caller_ap_node_opt footprint_access_path access_tree with
-            | Some (_, (caller_ap_trace, _)) ->
-                TraceDomain.join caller_ap_trace acc
-            | None ->
-                acc
+            let _, (caller_ap_trace, _) =
+              get_caller_ap_node_opt footprint_access_path access_tree
+            in
+            TraceDomain.join caller_ap_trace acc
           else acc
         in
         let {TraceDomain.Sources.footprint} = TraceDomain.sources callee_trace in
@@ -368,18 +377,20 @@ module Make (TaintSpecification : TaintSpec.S) = struct
         appended_trace
       in
       let add_to_caller_tree access_tree_acc callee_ap callee_trace =
-        match get_caller_ap_node_opt callee_ap access_tree_acc with
-        | Some (caller_ap, (caller_trace, caller_tree)) ->
-            let trace = instantiate_and_report callee_trace caller_trace access_tree_acc in
-            let pruned_trace =
-              if TraceDomain.Sources.Footprint.is_empty (TraceDomain.sources trace).footprint then
-                (* empty footprint; nothing else can flow into these sinks. so don't track them *)
-                TraceDomain.update_sinks trace TraceDomain.Sinks.empty
-              else trace
-            in
+        let caller_ap_opt, (caller_trace, caller_tree) =
+          get_caller_ap_node_opt callee_ap access_tree_acc
+        in
+        let trace = instantiate_and_report callee_trace caller_trace access_tree_acc in
+        let pruned_trace =
+          if TraceDomain.Sources.Footprint.is_empty (TraceDomain.sources trace).footprint then
+            (* empty footprint; nothing else can flow into these sinks. so don't track them *)
+            TraceDomain.update_sinks trace TraceDomain.Sinks.empty
+          else trace
+        in
+        match caller_ap_opt with
+        | Some caller_ap ->
             TaintDomain.add_node caller_ap (pruned_trace, caller_tree) access_tree_acc
         | None ->
-            ignore (instantiate_and_report callee_trace TraceDomain.empty access_tree_acc) ;
             access_tree_acc
       in
       TaintDomain.trace_fold add_to_caller_tree summary caller_access_tree

infer/tests/codetoanalyze/cpp/quandary/expressions.cpp

@@ -0,0 +1,57 @@
+/*
+ * Copyright (c) 2017 - present Facebook, Inc.
+ * All rights reserved.
+ *
+ * This source code is licensed under the BSD style license found in the
+ * LICENSE file in the root directory of this source tree. An additional grant
+ * of patent rights can be found in the PATENTS file in the same directory.
+ */
+
+extern int __infer_taint_source();
+extern void __infer_taint_sink(int);
+
+namespace expressions {
+
+void propagate_via_unop1_bad() {
+  int source = __infer_taint_source();
+  int laundered = ~source;
+  __infer_taint_sink(laundered);
+}
+
+void propagate_via_unop2_bad() {
+  int source = __infer_taint_source();
+  __infer_taint_sink(~source);
+}
+
+void propagate_via_binop1_bad() {
+  int source = __infer_taint_source();
+  int laundered = 5 + source % 7;
+  __infer_taint_sink(laundered);
+}
+
+void propagate_via_binop2_bad() {
+  int source1 = __infer_taint_source();
+  int source2 = __infer_taint_source();
+  int laundered = 1 + source1 / 2 + source2 * 7;
+  // should report twice
+  __infer_taint_sink(laundered);
+}
+
+void propagate_via_binop3_bad() {
+  int source = __infer_taint_source();
+  __infer_taint_sink(source - 3);
+}
+
+void call_sink_nested(int formal) { __infer_taint_sink(formal); }
+
+void propagate_via_binop_nested1_bad() {
+  int source = ~(__infer_taint_source());
+  call_sink_nested(source);
+}
+
+void propagate_via_binop_nested2_bad() {
+  int source = __infer_taint_source();
+  call_sink_nested(~source);
+}
+
+} // namespace expressions

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

@@ -57,6 +57,14 @@ codetoanalyze/cpp/quandary/execs.cpp, execs::callExecBad, 35, SHELL_INJECTION, [
 codetoanalyze/cpp/quandary/execs.cpp, execs::exec_flag_bad, 0, SHELL_INJECTION, [Return from __global_access,Call to execl]
 codetoanalyze/cpp/quandary/execs.cpp, execs::exec_flag_interproc_bad, 2, SHELL_INJECTION, [Return from __global_access with tainted data &return,Return from execs::return_global,Call to execl]
 codetoanalyze/cpp/quandary/execs.cpp, execs::sql_on_env_var_bad, 2, SQL_INJECTION, [Return from getenv,Call to __infer_sql_sink]
+codetoanalyze/cpp/quandary/expressions.cpp, expressions::propagate_via_binop1_bad, 3, QUANDARY_TAINT_ERROR, [Return from __infer_taint_source,Call to __infer_taint_sink]
+codetoanalyze/cpp/quandary/expressions.cpp, expressions::propagate_via_binop2_bad, 5, QUANDARY_TAINT_ERROR, [Return from __infer_taint_source,Call to __infer_taint_sink]
+codetoanalyze/cpp/quandary/expressions.cpp, expressions::propagate_via_binop2_bad, 5, QUANDARY_TAINT_ERROR, [Return from __infer_taint_source,Call to __infer_taint_sink]
+codetoanalyze/cpp/quandary/expressions.cpp, expressions::propagate_via_binop3_bad, 2, QUANDARY_TAINT_ERROR, [Return from __infer_taint_source,Call to __infer_taint_sink]
+codetoanalyze/cpp/quandary/expressions.cpp, expressions::propagate_via_binop_nested1_bad, 2, QUANDARY_TAINT_ERROR, [Return from __infer_taint_source,Call to expressions::call_sink_nested,Call to __infer_taint_sink]
+codetoanalyze/cpp/quandary/expressions.cpp, expressions::propagate_via_binop_nested2_bad, 2, QUANDARY_TAINT_ERROR, [Return from __infer_taint_source,Call to expressions::call_sink_nested,Call to __infer_taint_sink]
+codetoanalyze/cpp/quandary/expressions.cpp, expressions::propagate_via_unop1_bad, 3, QUANDARY_TAINT_ERROR, [Return from __infer_taint_source,Call to __infer_taint_sink]
+codetoanalyze/cpp/quandary/expressions.cpp, expressions::propagate_via_unop2_bad, 2, QUANDARY_TAINT_ERROR, [Return from __infer_taint_source,Call to __infer_taint_sink]
 codetoanalyze/cpp/quandary/files.cpp, files::read_file_call_exec_bad1, 5, SHELL_INJECTION, [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, SHELL_INJECTION, [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, SHELL_INJECTION, [Return from std::basic_istream<char,std::char_traits<char>>_getline,Call to execle]

infer/tests/codetoanalyze/cpp/quandary/unknown_code.cpp

@@ -7,6 +7,8 @@
  * of patent rights can be found in the PATENTS file in the same directory.
  */
 
+#include <map>
+#include <memory>
 #include <string>
 
 extern std::string __infer_taint_source();
@@ -15,6 +17,8 @@ extern std::string skip_value(std::string);
 extern std::string* skip_pointer(std::string);
 extern void skip_by_ref(std::string, std::string&);
 
+extern int of_string(std::string);
+
 namespace unknown_code {
 
 void direct_bad() {
@@ -54,4 +58,5 @@ void FN_via_skip_by_ref_bad() {
   skip_by_ref(source, laundered_by_ref);
   __infer_taint_sink(laundered_by_ref);
 }
+
 }