From 92011790c2449e725ad651ff16ad0a0985bf9f80 Mon Sep 17 00:00:00 2001 From: Sam Blackshear Date: Tue, 11 Apr 2017 04:48:01 -0700 Subject: [PATCH] [quandary] optimize handling of unknown code by adding notion of 'taintable types' Reviewed By: jeremydubreil Differential Revision: D4846886 fbshipit-source-id: d8364cc --- infer/src/checkers/Trace.ml | 4 +++ infer/src/checkers/Trace.mli | 2 ++ infer/src/quandary/ClangTaintAnalysis.ml | 2 ++ infer/src/quandary/JavaTaintAnalysis.ml | 15 ++++++++++ infer/src/quandary/TaintAnalysis.ml | 29 +++++++++++++++++-- infer/src/quandary/TaintSpec.ml | 3 ++ infer/src/unit/TaintTests.ml | 1 + .../codetoanalyze/java/quandary/Strings.java | 15 ++++++++++ .../codetoanalyze/java/quandary/issues.exp | 1 + 9 files changed, 69 insertions(+), 3 deletions(-) diff --git a/infer/src/checkers/Trace.ml b/infer/src/checkers/Trace.ml index f7908eaf5..918857372 100644 --- a/infer/src/checkers/Trace.ml +++ b/infer/src/checkers/Trace.ml @@ -72,6 +72,8 @@ module type S = sig (** add a sink to the current trace. *) val add_sink : Sink.t -> t -> t + val update_sources : t -> Sources.t -> t + val update_sinks : t -> Sinks.t -> t (** append the trace for given call site to the current caller trace *) @@ -339,6 +341,8 @@ module Make (Spec : Spec) = struct let sinks = Sinks.add sink t.sinks in { t with sinks; } + let update_sources t sources = { t with sources } + let update_sinks t sinks = { t with sinks } (** compute caller_trace + callee_trace *) diff --git a/infer/src/checkers/Trace.mli b/infer/src/checkers/Trace.mli index 5cc8d40ec..c331d8be6 100644 --- a/infer/src/checkers/Trace.mli +++ b/infer/src/checkers/Trace.mli @@ -73,6 +73,8 @@ module type S = sig (** add a sink to the current trace. *) val add_sink : Sink.t -> t -> t + val update_sources : t -> Sources.t -> t + (** replace sinks with new ones *) val update_sinks : t -> Sinks.t -> t diff --git a/infer/src/quandary/ClangTaintAnalysis.ml b/infer/src/quandary/ClangTaintAnalysis.ml index 8053670e2..34a4d9d9b 100644 --- a/infer/src/quandary/ClangTaintAnalysis.ml +++ b/infer/src/quandary/ClangTaintAnalysis.ml @@ -25,4 +25,6 @@ include let handle_unknown_call _ _ _ _ = [] + + let is_taintable_type _ = true end) diff --git a/infer/src/quandary/JavaTaintAnalysis.ml b/infer/src/quandary/JavaTaintAnalysis.ml index 422d5a614..534fcc305 100644 --- a/infer/src/quandary/JavaTaintAnalysis.ml +++ b/infer/src/quandary/JavaTaintAnalysis.ml @@ -68,4 +68,19 @@ include [] | pname -> failwithf "Non-Java procname %a in Java analysis@." Typ.Procname.pp pname + + let is_taintable_type = function + | Typ.Tptr (Tstruct (JavaClass typename), _) | Tstruct (JavaClass typename) -> + begin + match Mangled.to_string_full typename with + | "android.content.Intent" + | "android.net.Uri" + | "java.lang.String" + | "java.net.URI" -> + true + | _ -> + false + end + | _ -> + false end) diff --git a/infer/src/quandary/TaintAnalysis.ml b/infer/src/quandary/TaintAnalysis.ml index 4af1a0789..7d6553465 100644 --- a/infer/src/quandary/TaintAnalysis.ml +++ b/infer/src/quandary/TaintAnalysis.ml @@ -367,6 +367,15 @@ module Make (TaintSpecification : TaintSpec.S) = struct | Sil.Call (ret, Const (Cfun called_pname), actuals, callee_loc, call_flags) -> let handle_unknown_call callee_pname astate = + let is_variadic = match callee_pname with + | Typ.Procname.Java pname -> + begin + match List.rev (Typ.Procname.java_get_parameters pname) with + | (_, "java.lang.Object[]") :: _ -> true + | _ -> false + end + | _ -> false in + let should_taint_typ typ = is_variadic || TaintSpecification.is_taintable_type typ in let exp_join_traces trace_acc (exp, typ) = match exp_get_node ~abstracted:true exp typ astate proc_data with | Some (trace, _) -> TraceDomain.join trace trace_acc @@ -375,9 +384,23 @@ module Make (TaintSpecification : TaintSpec.S) = struct let initial_trace = access_path_get_trace access_path astate.access_tree proc_data in let trace_with_propagation = List.fold ~f:exp_join_traces ~init:initial_trace actuals in - let access_tree = - TaintDomain.add_trace access_path trace_with_propagation astate.access_tree in - { astate with access_tree; } in + let filtered_sources = + TraceDomain.Sources.filter (fun source -> + match TraceDomain.Source.get_footprint_access_path source with + | Some access_path -> + Option.exists + (AccessPath.Raw.get_typ (AccessPath.extract access_path) proc_data.tenv) + ~f:should_taint_typ + | None -> true) + (TraceDomain.sources trace_with_propagation) in + if TraceDomain.Sources.is_empty filtered_sources + then + astate + else + let trace' = TraceDomain.update_sources trace_with_propagation filtered_sources in + let access_tree = + TaintDomain.add_trace access_path trace' astate.access_tree in + { astate with Domain.access_tree; } in let handle_unknown_call_ astate_acc propagation = match propagation, actuals, ret with | _, [], _ -> diff --git a/infer/src/quandary/TaintSpec.ml b/infer/src/quandary/TaintSpec.ml index 30ff6be9c..97a0956ef 100644 --- a/infer/src/quandary/TaintSpec.ml +++ b/infer/src/quandary/TaintSpec.ml @@ -25,6 +25,9 @@ module type S = sig val handle_unknown_call : Typ.Procname.t -> Typ.t option -> (Exp.t * Typ.t) list -> Tenv.t -> handle_unknown list + (** return true if the given typ can be tainted *) + val is_taintable_type : Typ.t -> bool + val to_summary_access_tree : AccessTree.t -> QuandarySummary.AccessTree.t val of_summary_access_tree : QuandarySummary.AccessTree.t -> AccessTree.t diff --git a/infer/src/unit/TaintTests.ml b/infer/src/unit/TaintTests.ml index 647519743..ff2de0281 100644 --- a/infer/src/unit/TaintTests.ml +++ b/infer/src/unit/TaintTests.ml @@ -46,6 +46,7 @@ module MockTaintAnalysis = TaintAnalysis.Make(struct let of_summary_access_tree _ = assert false let to_summary_access_tree _ = assert false let handle_unknown_call _ _ _ _ = [] + let is_taintable_type _ = true end) module TestInterpreter = AnalyzerTester.Make (ProcCfg.Normal) (MockTaintAnalysis.TransferFunctions) diff --git a/infer/tests/codetoanalyze/java/quandary/Strings.java b/infer/tests/codetoanalyze/java/quandary/Strings.java index 6a2e415d5..6b4ddca67 100644 --- a/infer/tests/codetoanalyze/java/quandary/Strings.java +++ b/infer/tests/codetoanalyze/java/quandary/Strings.java @@ -63,4 +63,19 @@ public class Strings { InferTaint.inferSensitiveSink(formatter.toString()); } + void viaStringFormatVarArgsDirectBad() { + Object source = InferTaint.inferSecretSource(); + String tainted = String.format("%s%s", "hi", source); + InferTaint.inferSensitiveSink(tainted); + } + + void viaStringFormatVarArgsIndirect(Object param) { + String tainted = String.format("%s%s", "hi", param); + InferTaint.inferSensitiveSink(tainted); + } + + void FN_viaStringFormatVarArgsIndirectBad() { + viaStringFormatVarArgsIndirect(InferTaint.inferSecretSource()); + } + } diff --git a/infer/tests/codetoanalyze/java/quandary/issues.exp b/infer/tests/codetoanalyze/java/quandary/issues.exp index 97e67d475..497520a95 100644 --- a/infer/tests/codetoanalyze/java/quandary/issues.exp +++ b/infer/tests/codetoanalyze/java/quandary/issues.exp @@ -171,6 +171,7 @@ codetoanalyze/java/quandary/Strings.java, void Strings.viaStringBufferIgnoreRetu codetoanalyze/java/quandary/Strings.java, void Strings.viaStringBuilderBad(), 3, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),call to void InferTaint.inferSensitiveSink(Object)] codetoanalyze/java/quandary/Strings.java, void Strings.viaStringBuilderIgnoreReturnBad(), 5, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),call to void InferTaint.inferSensitiveSink(Object)] codetoanalyze/java/quandary/Strings.java, void Strings.viaStringBuilderSugarBad(), 2, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),call to void InferTaint.inferSensitiveSink(Object)] +codetoanalyze/java/quandary/Strings.java, void Strings.viaStringFormatVarArgsDirectBad(), 3, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),call to void InferTaint.inferSensitiveSink(Object)] codetoanalyze/java/quandary/TaintedFormals.java, void TaintedFormals.callTaintedContextBad1(String), 2, QUANDARY_TAINT_ERROR, [return from Object TaintedFormals.taintedContextBad(String),return from Object TaintedFormals.taintedContextBad(String),call to void InferTaint.inferSensitiveSink(Object)] codetoanalyze/java/quandary/TaintedFormals.java, void TaintedFormals.callTaintedContextBad2(), 1, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),call to void TaintedFormals.taintedContextBad(String,Intent,Integer),call to ComponentName ContextWrapper.startService(Intent)] codetoanalyze/java/quandary/TaintedFormals.java, void TaintedFormals.taintedContextBad(String,Intent,Integer), 3, QUANDARY_TAINT_ERROR, [return from void TaintedFormals.taintedContextBad(String,Intent,Integer),call to void InferTaint.inferSensitiveSink(Object)]