[quandary] improved handling of unknown code in C++

Summary: In particular, the heuristics for propagating taint via unknown code needs to be aware of the frontend's trick of introducing dummy return variables.

Reviewed By: mbouaziz

Differential Revision: D5046345

fbshipit-source-id: da87665

infer/src/quandary/ClangTaintAnalysis.ml

@@ -23,11 +23,21 @@ include
       | QuandarySummary.AccessTree.Clang tree -> tree
       | _ -> assert false
 
-    let handle_unknown_call _ ret_typ_opt actuals _ = match ret_typ_opt, actuals with
+    let handle_unknown_call _ ret_typ_opt actuals _ =
+      match ret_typ_opt, List.rev actuals with
       | Some _, _ ->
           (* propagate taint from actuals to return value *)
           [TaintSpec.Propagate_to_return]
-      | None, [] -> []
+      | None, [] ->
+          []
+      | None,
+        HilExp.AccessPath ((Var.ProgramVar pvar, { desc=Typ.Tptr (_, Typ.Pk_pointer) }), []) :: _
+        when Pvar.is_frontend_tmp pvar ->
+          (* no return value, but the frontend has introduced a dummy return variable and will
+             assign the return value to this variable. So propagate taint to the dummy return
+             variable *)
+          let actual_index = List.length actuals - 1 in
+          [TaintSpec.Propagate_to_actual actual_index]
       | None, _ ->
           (* no return value; propagate taint from actuals to receiver *)
           [TaintSpec.Propagate_to_receiver]

infer/src/quandary/TaintAnalysis.ml

@@ -346,11 +346,19 @@ module Make (TaintSpecification : TaintSpec.S) = struct
                 AccessPath receiver_ap :: (_ :: _ as other_actuals),
                 _ ->
                   propagate_to_access_path (AccessPath.Exact receiver_ap) other_actuals astate_acc
+              | TaintSpec.Propagate_to_actual actual_index, _, _ ->
+                  begin
+                    match List.nth actuals actual_index with
+                    | Some (HilExp.AccessPath actual_ap) ->
+                        propagate_to_access_path (AccessPath.Exact actual_ap) actuals astate_acc
+                    | _ ->
+                        astate_acc
+                  end
               | _ ->
                   astate_acc in
 
             match Typ.Procname.get_method callee_pname with
-            | "operator=" when Typ.Procname.is_c_method callee_pname ->
+            | "operator=" when not (Typ.Procname.is_java callee_pname) ->
                 (* treat unknown calls to C++ operator= as assignment *)
                 begin
                   match actuals with
@@ -389,7 +397,7 @@ module Make (TaintSpecification : TaintSpec.S) = struct
                       "Warning: %a is marked as a source, but has no return value"
                       Typ.Procname.pp callee_pname;
                     astate_with_sink in
-                  if Typ.Procname.is_c_method called_pname
+                  if not (Typ.Procname.is_java callee_pname)
                   then
                     (* the C++ frontend handles returns of non-pointers by adding a dummy
                        pass-by-reference variable as the last actual, then returning the value
@@ -442,10 +450,7 @@ module Make (TaintSpecification : TaintSpec.S) = struct
     AbstractInterpreter.Make (ProcCfg.Exceptional) (LowerHil.Make(TransferFunctions))
 
   let make_summary { ProcData.pdesc; extras={ formal_map; } } access_tree =
-    let is_java = match Procdesc.get_proc_name pdesc with
-      | Java _ -> true
-      | _ -> false in
-
+    let is_java = Typ.Procname.is_java (Procdesc.get_proc_name pdesc) in
     (* if a trace has footprint sources, attach them to the appropriate footprint var *)
     let access_tree' =
       TaintDomain.fold

infer/src/quandary/TaintSpec.ml

@@ -13,8 +13,13 @@ open! IStd
     summaries *)
 
 type handle_unknown =
-  | Propagate_to_return
+  | Propagate_to_actual of int
+  (** Propagate taint from all actuals to the actual with the given index *)
   | Propagate_to_receiver
+  (** Propagate taint from all non-receiver actuals to the receiver actual *)
+  | Propagate_to_return
+  (** Propagate taint from all actuals to the return value *)
+
 
 module type S = sig
   module Trace : Trace.S

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

@@ -39,3 +39,7 @@ codetoanalyze/cpp/quandary/pointers.cpp, pointers::assign_pointer_pass_to_sink_b
 codetoanalyze/cpp/quandary/pointers.cpp, pointers::assign_source_by_reference_bad1, 3, QUANDARY_TAINT_ERROR, [return from __infer_taint_source,return from pointers::assign_source_by_reference,call to __infer_taint_sink]
 codetoanalyze/cpp/quandary/pointers.cpp, pointers::assign_source_by_reference_bad2, 2, QUANDARY_TAINT_ERROR, [return from __infer_taint_source,return from pointers::assign_source_by_reference,call to __infer_taint_sink]
 codetoanalyze/cpp/quandary/pointers.cpp, pointers::assign_source_by_reference_bad3, 3, QUANDARY_TAINT_ERROR, [return from __infer_taint_source,return from pointers::assign_source_by_reference,return from pointers::call_assign_source_by_reference,call to __infer_taint_sink]
+codetoanalyze/cpp/quandary/unknown_code.cpp, unknown_code::direct_bad, 2, QUANDARY_TAINT_ERROR, [return from __infer_taint_source,call to __infer_taint_sink]
+codetoanalyze/cpp/quandary/unknown_code.cpp, unknown_code::skip_indirect_bad, 3, QUANDARY_TAINT_ERROR, [return from __infer_taint_source,flow through unknown_code::skip_indirect,call to __infer_taint_sink]
+codetoanalyze/cpp/quandary/unknown_code.cpp, unknown_code::skip_pointer_bad, 3, QUANDARY_TAINT_ERROR, [return from __infer_taint_source,call to __infer_taint_sink]
+codetoanalyze/cpp/quandary/unknown_code.cpp, unknown_code::skip_value_bad, 3, QUANDARY_TAINT_ERROR, [return from __infer_taint_source,call to __infer_taint_sink]

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

@@ -0,0 +1,57 @@
+/*
+ * Copyright (c) 2016 - 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.
+ */
+
+#include <string>
+
+extern std::string __infer_taint_source();
+extern void __infer_taint_sink(std::string);
+extern std::string skip_value(std::string);
+extern std::string* skip_pointer(std::string);
+extern void skip_by_ref(std::string, std::string&);
+
+namespace unknown_code {
+
+void direct_bad() {
+  auto source = __infer_taint_source();
+  __infer_taint_sink(source);
+}
+
+void skip_value_bad() {
+  auto source = __infer_taint_source();
+  auto laundered_source = skip_value(source);
+  __infer_taint_sink(laundered_source);
+}
+
+void skip_pointer_bad() {
+  auto source = __infer_taint_source();
+  auto laundered_source = skip_pointer(source);
+  __infer_taint_sink(*laundered_source);
+}
+
+std::string skip_indirect(std::string formal) {
+  auto skipped_pointer = skip_pointer(formal);
+  return skip_value(*skipped_pointer);
+}
+
+void skip_indirect_bad() {
+  auto source = __infer_taint_source();
+  auto laundered_source = skip_indirect(source);
+  __infer_taint_sink(laundered_source);
+}
+
+// for now, we don't have any heuristics for guessing that laundered_by_ref is
+// assigned by ref in
+// the skipped function
+void FN_via_skip_by_ref_bad() {
+  auto source = __infer_taint_source();
+  std::string laundered_by_ref;
+  skip_by_ref(source, laundered_by_ref);
+  __infer_taint_sink(laundered_by_ref);
+}
+}