From 2b0335f32bf0adfa45b437500ba04d77900d932d Mon Sep 17 00:00:00 2001 From: Sam Blackshear Date: Tue, 14 Nov 2017 09:58:07 -0800 Subject: [PATCH] [quandary] track sanitizers applied in trace domain Reviewed By: mbouaziz Differential Revision: D6302845 fbshipit-source-id: c104a73 --- infer/src/checkers/SinkTrace.ml | 2 +- infer/src/checkers/Trace.ml | 52 ++++++++++++++++++++++------- infer/src/checkers/Trace.mli | 13 ++++++-- infer/src/quandary/ClangTrace.ml | 5 ++- infer/src/quandary/JavaTrace.ml | 6 +++- infer/src/quandary/TaintAnalysis.ml | 7 ++-- infer/src/unit/TaintTests.ml | 2 +- infer/src/unit/TraceTests.ml | 2 +- 8 files changed, 67 insertions(+), 22 deletions(-) diff --git a/infer/src/checkers/SinkTrace.ml b/infer/src/checkers/SinkTrace.ml index 20d139cb7..5270ea9dd 100644 --- a/infer/src/checkers/SinkTrace.ml +++ b/infer/src/checkers/SinkTrace.ml @@ -43,7 +43,7 @@ module Make (TraceElem : TraceElem.S) = struct module Sanitizer = Sanitizer.Dummy module Sink = MakeSink (TraceElem) - let get_report _ _ = Some IssueType.do_not_report + let get_report _ _ _ = Some IssueType.do_not_report end) type sink_path = Passthroughs.t * (Sink.t * Passthroughs.t) list diff --git a/infer/src/checkers/Trace.ml b/infer/src/checkers/Trace.ml index 9b269e4e5..cb09c5f5f 100644 --- a/infer/src/checkers/Trace.ml +++ b/infer/src/checkers/Trace.ml @@ -18,7 +18,7 @@ module type Spec = sig module Sanitizer : Sanitizer.S - val get_report : Source.t -> Sink.t -> IssueType.t option + val get_report : Source.t -> Sink.t -> Sanitizer.t list -> IssueType.t option end module type S = sig @@ -37,7 +37,9 @@ module type S = sig module Footprint : module type of AccessTree.PathSet (FootprintConfig) - type astate = {known: Known.astate; footprint: Footprint.astate} + module Sanitizers : module type of AbstractDomain.FiniteSet (Sanitizer) + + type astate = {known: Known.astate; footprint: Footprint.astate; sanitizers: Sanitizers.astate} type t = astate @@ -110,6 +112,9 @@ module type S = sig val add_sink : Sink.t -> t -> t (** add a sink to the current trace. *) + val add_sanitizer : Sanitizer.t -> t -> t + (** add a sanitizer to the current trace *) + val update_sources : t -> Sources.t -> t val update_sinks : t -> Sinks.t -> t @@ -172,8 +177,9 @@ module Make (Spec : Spec) = struct module Known = AbstractDomain.FiniteSet (Source) module FootprintConfig = AccessTree.DefaultConfig module Footprint = AccessTree.PathSet (FootprintConfig) + module Sanitizers = AbstractDomain.FiniteSet (Sanitizer) - type astate = {known: Known.astate; footprint: Footprint.astate} + type astate = {known: Known.astate; footprint: Footprint.astate; sanitizers: Sanitizers.astate} type t = astate @@ -181,6 +187,7 @@ module Make (Spec : Spec) = struct if phys_equal lhs rhs then true else Known.( <= ) ~lhs:lhs.known ~rhs:rhs.known && Footprint.( <= ) ~lhs:lhs.footprint ~rhs:rhs.footprint + && Sanitizers.( <= ) ~lhs:lhs.sanitizers ~rhs:rhs.sanitizers let join astate1 astate2 = @@ -188,7 +195,8 @@ module Make (Spec : Spec) = struct else let known = Known.join astate1.known astate2.known in let footprint = Footprint.join astate1.footprint astate2.footprint in - {known; footprint} + let sanitizers = Sanitizers.join astate1.sanitizers astate2.sanitizers in + {known; footprint; sanitizers} let widen ~prev ~next ~num_iters = @@ -196,18 +204,26 @@ module Make (Spec : Spec) = struct else let known = Known.widen ~prev:prev.known ~next:next.known ~num_iters in let footprint = Footprint.widen ~prev:prev.footprint ~next:next.footprint ~num_iters in - {known; footprint} + let sanitizers = Sanitizers.widen ~prev:prev.sanitizers ~next:next.sanitizers ~num_iters in + {known; footprint; sanitizers} - let pp fmt {known; footprint} = + let pp fmt {known; footprint; sanitizers} = + let pp_sanitizers fmt sanitizers = + if not (Sanitizers.is_empty sanitizers) then + F.fprintf fmt " + Sanitizers(%a)" Sanitizers.pp sanitizers + in if Known.is_empty known then if Footprint.is_empty footprint then F.fprintf fmt "{}" - else F.fprintf fmt "Footprint(%a)" Footprint.pp footprint - else F.fprintf fmt "%a + Footprint(%a)" Known.pp known Footprint.pp footprint + else F.fprintf fmt "Footprint(%a)%a" Footprint.pp footprint pp_sanitizers sanitizers + else + F.fprintf fmt "%a + Footprint(%a)%a" Known.pp known Footprint.pp footprint pp_sanitizers + sanitizers - let empty = {known= Known.empty; footprint= Footprint.empty} + let empty = {known= Known.empty; footprint= Footprint.empty; sanitizers= Sanitizers.empty} + (* note: empty known/footprint implies empty sanitizers *) let is_empty {known; footprint} = Known.is_empty known && Footprint.BaseMap.is_empty footprint let of_footprint access_path = @@ -311,7 +327,9 @@ module Make (Spec : Spec) = struct let report_source source sinks acc0 = let report_one sink acc = if should_report_at_site source sink then - match Spec.get_report source sink with + match + Spec.get_report source sink (Sources.Sanitizers.elements t.sources.sanitizers) + with | Some issue -> { issue ; path_source= Known source @@ -514,6 +532,12 @@ module Make (Spec : Spec) = struct {t with sinks} + let add_sanitizer sanitizer t = + let sanitizers = Sources.Sanitizers.add sanitizer t.sources.sanitizers in + let sources = {t.sources with sanitizers} in + {t with sources} + + let update_sources t sources = {t with sources} let update_sinks t sinks = {t with sinks} @@ -524,10 +548,14 @@ module Make (Spec : Spec) = struct let append caller_trace callee_trace callee_site = if is_empty callee_trace then caller_trace else + let sanitizers = + Sources.Sanitizers.join callee_trace.sources.sanitizers caller_trace.sources.sanitizers + in let non_footprint_callee_sources = callee_trace.sources.known in let sources = if Sources.Known.subset non_footprint_callee_sources caller_trace.sources.known then - caller_trace.sources + if phys_equal sanitizers caller_trace.sources.sanitizers then caller_trace.sources + else {caller_trace.sources with Sources.sanitizers} else let known = List.map @@ -535,7 +563,7 @@ module Make (Spec : Spec) = struct (Sources.Known.elements non_footprint_callee_sources) |> Sources.Known.of_list |> Sources.Known.union caller_trace.sources.known in - {caller_trace.sources with Sources.known} + {caller_trace.sources with Sources.known; sanitizers} in let sinks = if Sinks.subset callee_trace.sinks caller_trace.sinks then caller_trace.sinks diff --git a/infer/src/checkers/Trace.mli b/infer/src/checkers/Trace.mli index d44cbd407..27393f3c0 100644 --- a/infer/src/checkers/Trace.mli +++ b/infer/src/checkers/Trace.mli @@ -18,8 +18,9 @@ module type Spec = sig module Sanitizer : Sanitizer.S - val get_report : Source.t -> Sink.t -> IssueType.t option - (** return Some(issue) if the source and sink match, None otherwise *) + val get_report : Source.t -> Sink.t -> Sanitizer.t list -> IssueType.t option + (** return Some(issue) a trace from source to sink passing through the given sanitizers should be + reported, None otherwise *) end module type S = sig @@ -40,7 +41,10 @@ module type S = sig (** Set of access paths representing the sources that may flow in from the caller *) module Footprint : module type of AccessTree.PathSet (FootprintConfig) - type astate = {known: Known.astate; footprint: Footprint.astate} + (** Set of sanitizers that have been applied to these sources *) + module Sanitizers : module type of AbstractDomain.FiniteSet (Sanitizer) + + type astate = {known: Known.astate; footprint: Footprint.astate; sanitizers: Sanitizers.astate} type t = astate @@ -120,6 +124,9 @@ module type S = sig val add_sink : Sink.t -> t -> t (** add a sink to the current trace. *) + val add_sanitizer : Sanitizer.t -> t -> t + (** add a sanitizer to the current trace *) + val update_sources : t -> Sources.t -> t val update_sinks : t -> Sinks.t -> t diff --git a/infer/src/quandary/ClangTrace.ml b/infer/src/quandary/ClangTrace.ml index 72fa3797b..82df4f04e 100644 --- a/infer/src/quandary/ClangTrace.ml +++ b/infer/src/quandary/ClangTrace.ml @@ -324,7 +324,7 @@ include Trace.Make (struct module Sink = CppSink module Sanitizer = CppSanitizer - let get_report source sink = + let get_report source sink sanitizers = (* TODO: make this accept structs/objects too, but not primitive types or enumes *) (* using this to match custom string wrappers such as folly::StringPiece *) let is_stringy typ = @@ -333,6 +333,9 @@ include Trace.Make (struct || String.is_substring ~substring:"char*" lowercase_typ in match (Source.kind source, Sink.kind sink) with + | _ when not (List.is_empty sanitizers) -> + (* assume any sanitizer clears all forms of taint *) + None | Endpoint (_, typ), (ShellExec | SQL) -> (* remote code execution if the caller of the endpoint doesn't sanitize *) Option.some_if (is_stringy typ) IssueType.remote_code_execution_risk diff --git a/infer/src/quandary/JavaTrace.ml b/infer/src/quandary/JavaTrace.ml index 9672bfaa5..cdaf11e89 100644 --- a/infer/src/quandary/JavaTrace.ml +++ b/infer/src/quandary/JavaTrace.ml @@ -381,8 +381,11 @@ include Trace.Make (struct module Sink = JavaSink module Sanitizer = JavaSanitizer - let get_report source sink = + let get_report source sink sanitizers = match (Source.kind source, Sink.kind sink) with + | _ when not (List.is_empty sanitizers) -> + (* assume any sanitizer clears all forms of taint *) + L.d_strln "non-empty sanitizers!" ; None | PrivateData, Logging (* logging private data issue *) | Intent, StartComponent @@ -407,6 +410,7 @@ include Trace.Make (struct (* not a security issue, but useful for debugging flows from resource IDs to inflation *) Some IssueType.quandary_taint_error | Other, _ | _, Other -> + L.d_strln (F.asprintf "have %d sanitizers" (List.length sanitizers)) ; (* for testing purposes, Other matches everything *) Some IssueType.quandary_taint_error | _ -> diff --git a/infer/src/quandary/TaintAnalysis.ml b/infer/src/quandary/TaintAnalysis.ml index f0602ed64..64da44f24 100644 --- a/infer/src/quandary/TaintAnalysis.ml +++ b/infer/src/quandary/TaintAnalysis.ml @@ -640,8 +640,11 @@ module Make (TaintSpecification : TaintSpec.S) = struct astate_with_summary | Some ret_base -> match TraceDomain.Sanitizer.get callee_pname with - | Some _ -> - TaintDomain.BaseMap.remove ret_base astate_with_summary + | Some sanitizer -> + let ret_ap = AccessPath.Abs.Exact (ret_base, []) in + let ret_trace = access_path_get_trace ret_ap astate_with_summary proc_data in + let ret_trace' = TraceDomain.add_sanitizer sanitizer ret_trace in + TaintDomain.add_trace ret_ap ret_trace' astate_with_summary | None -> astate_with_summary in diff --git a/infer/src/unit/TaintTests.ml b/infer/src/unit/TaintTests.ml index ae49fcfab..07ed31fd9 100644 --- a/infer/src/unit/TaintTests.ml +++ b/infer/src/unit/TaintTests.ml @@ -41,7 +41,7 @@ module MockTrace = Trace.Make (struct module Sanitizer = Sanitizer.Dummy - let get_report _ _ = None + let get_report _ _ _ = None end) module MockTaintAnalysis = TaintAnalysis.Make (struct diff --git a/infer/src/unit/TraceTests.ml b/infer/src/unit/TraceTests.ml index 4ae7caddc..1391c8fe6 100644 --- a/infer/src/unit/TraceTests.ml +++ b/infer/src/unit/TraceTests.ml @@ -81,7 +81,7 @@ module MockTrace = Trace.Make (struct module Sink = MockSink module Sanitizer = Sanitizer.Dummy - let get_report source sink = + let get_report source sink _ = if [%compare.equal : MockTraceElem.t] (Source.kind source) (Sink.kind sink) then Some IssueType.quandary_taint_error else None