diff --git a/infer/src/quandary/ClangTaintAnalysis.ml b/infer/src/quandary/ClangTaintAnalysis.ml index 33cb4b848..d1ddb9d1c 100644 --- a/infer/src/quandary/ClangTaintAnalysis.ml +++ b/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] diff --git a/infer/src/quandary/TaintAnalysis.ml b/infer/src/quandary/TaintAnalysis.ml index 2e9c1d4f6..e3d497004 100644 --- a/infer/src/quandary/TaintAnalysis.ml +++ b/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 diff --git a/infer/src/quandary/TaintSpec.ml b/infer/src/quandary/TaintSpec.ml index 00e3418ae..e23bb6854 100644 --- a/infer/src/quandary/TaintSpec.ml +++ b/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 diff --git a/infer/tests/codetoanalyze/cpp/quandary/issues.exp b/infer/tests/codetoanalyze/cpp/quandary/issues.exp index dd6e5acfa..70836fc72 100644 --- a/infer/tests/codetoanalyze/cpp/quandary/issues.exp +++ b/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] diff --git a/infer/tests/codetoanalyze/cpp/quandary/unknown_code.cpp b/infer/tests/codetoanalyze/cpp/quandary/unknown_code.cpp new file mode 100644 index 000000000..dbc1ff964 --- /dev/null +++ b/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 + +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); +} +}