[quandary] add indexes to sinks

Summary:
First step toward addressing bad traces that happen in examples like

```
  void sourceMethod() {
    Obj source = (Obj) InferTaint.inferSecretSource();
    callSameSink(null, source); // index: 1
  }

  void callSameSink(Obj o1, Obj o2) {
    callMySink1(o1); // flows via o1 ~= index 0, don't expand
    callMySink2(o2); // flows via o2 ~= index 1, can expand
  }

  void callMySink1(Obj o) {
    ... // maybe interesting something happens here that doesn't happen in callMySink2
    InferTaint.inferSensitiveSink(o); // flows via o ~= index 0, can expand
  }

  void callMySink2(Obj o) {
    InferTaint.inferSensitiveSink(o); // flows via o ~= index 0, can expand
  }
```

The issue is that when we recreate a trace to the sink starting from `sourceMethod`, we don't know which of the calls to `callMySink` to expand/include in the trace.
If we expand the call to `callMySink(o1)`, we'll get a bogus trace.
In this example that's not such a big deal, but imagine the case where the first call to `callMySink` is a different function that transitively calls the sink through some long and confusing path.

Remembering the index at which taint flows into each sink will let us choose which sinks are safe to expand.
This diff just adds indexes to the API; it's not actually propagating the index info or using it during expansion yet.

Reviewed By: jeremydubreil

Differential Revision: D5170563

fbshipit-source-id: ba4b096
master
Sam Blackshear 8 years ago committed by Facebook Github Bot
parent 2089cd8a34
commit 97bf3324c8

@ -15,23 +15,15 @@ module L = Logging
module type Kind = sig module type Kind = sig
include TraceElem.Kind include TraceElem.Kind
(** return the parameter index and sink kind for the given call site with the given actuals *) val get : Typ.Procname.t -> HilExp.t list -> Tenv.t -> (t * IntSet.t) option
val get : Typ.Procname.t -> HilExp.t list -> Tenv.t -> (t * int) list
end end
module type S = sig module type S = sig
include TraceElem.S include TraceElem.S
type parameter = val get : CallSite.t -> HilExp.t list -> Tenv.t -> t option
{
sink : t; val indexes : t -> IntSet.t
(** sink type of the parameter *)
index : int;
(** index of the parameter *)
}
(** return the parameter index and sink kind for the given call site with the given actuals *)
val get : CallSite.t -> HilExp.t list -> Tenv.t -> parameter list
end end
module Make (Kind : Kind) = struct module Make (Kind : Kind) = struct
@ -41,32 +33,25 @@ module Make (Kind : Kind) = struct
{ {
kind : Kind.t; kind : Kind.t;
site : CallSite.t; site : CallSite.t;
indexes : IntSet.t;
} [@@deriving compare] } [@@deriving compare]
type parameter =
{
sink : t;
(** sink type of the parameter *)
index : int;
(** index of the parameter *)
}
let kind t = let kind t =
t.kind t.kind
let call_site t = let call_site t =
t.site t.site
let make kind site = let indexes t =
{ kind; site; } t.indexes
let make_sink_param sink index = let make ?(indexes=IntSet.empty) kind site =
{ sink; index; } { kind; site; indexes; }
let get site actuals tenv = let get site actuals tenv =
List.map match Kind.get (CallSite.pname site) actuals tenv with
~f:(fun (kind, index) -> make_sink_param (make kind site) index) | Some (kind, indexes) -> Some { kind; site; indexes; }
(Kind.get (CallSite.pname site) actuals tenv) | None -> None
let with_callsite t callee_site = let with_callsite t callee_site =
{ t with site = callee_site; } { t with site = callee_site; }

@ -12,23 +12,18 @@ open! IStd
module type Kind = sig module type Kind = sig
include TraceElem.Kind include TraceElem.Kind
(** return the parameter index and sink kind for the given call site with the given actuals *) (** return Some kind if the given procname/actuals are a sink, None otherwise *)
val get : Typ.Procname.t -> HilExp.t list -> Tenv.t -> (t * int) list val get : Typ.Procname.t -> HilExp.t list -> Tenv.t -> (t * IntSet.t) option
end end
module type S = sig module type S = sig
include TraceElem.S include TraceElem.S
type parameter = (** return Some sink if the given call site/actuals are a sink, None otherwise *)
{ val get : CallSite.t -> HilExp.t list -> Tenv.t -> t option
sink : t;
(** sink type of the parameter *)
index : int;
(** index of the parameter *)
}
(** return the parameter index and sink kind for the given call site with the given actuals *) (** return the indexes where taint can flow into the sink *)
val get : CallSite.t -> HilExp.t list -> Tenv.t -> parameter list val indexes : t -> IntSet.t
end end
module Make (Kind : Kind) : S with module Kind = Kind module Make (Kind : Kind) : S with module Kind = Kind

@ -32,9 +32,9 @@ end
module MakeSink(TraceElem : TraceElem.S) = struct module MakeSink(TraceElem : TraceElem.S) = struct
include TraceElem include TraceElem
type parameter = { sink : t; index : int; }
let get _ _ _ = [] let get _ _ _ = None
let indexes _ = IntSet.empty
end end
module Make (TraceElem : TraceElem.S) = struct module Make (TraceElem : TraceElem.S) = struct

@ -35,7 +35,7 @@ module TraceElem = struct
let kind { kind; } = snd kind let kind { kind; } = snd kind
let make kind site = { kind = (`Call, kind); site; } let make ?indexes:_ kind site = { kind = (`Call, kind); site; }
let with_callsite { kind=(_, kind); } site = { kind=(`Call, kind); site; } let with_callsite { kind=(_, kind); } site = { kind=(`Call, kind); site; }

@ -85,7 +85,7 @@ module Make (Kind : Kind) = struct
| Normal kind -> kind | Normal kind -> kind
| Footprint _ -> Kind.unknown | Footprint _ -> Kind.unknown
let make kind site = let make ?indexes:_ kind site =
{ site; kind = Normal kind; } { site; kind = Normal kind; }
let make_footprint ap pdesc = let make_footprint ap pdesc =
@ -135,7 +135,7 @@ module Dummy = struct
let kind t = t let kind t = t
let make kind _ = kind let make ?indexes:_ kind _ = kind
let pp _ () = () let pp _ () = ()

@ -48,7 +48,7 @@ module TraceElem = struct
let kind { kind; } = kind let kind { kind; } = kind
let make kind site = { kind; site; } let make ?indexes:_ kind site = { kind; site; }
let with_callsite t site = { t with site; } let with_callsite t site = { t with site; }

@ -25,7 +25,7 @@ module type S = sig
val call_site : t -> CallSite.t val call_site : t -> CallSite.t
val kind : t -> Kind.t val kind : t -> Kind.t
val make : Kind.t -> CallSite.t -> t val make : ?indexes:IntSet.t -> Kind.t -> CallSite.t -> t
val with_callsite : t -> CallSite.t -> t val with_callsite : t -> CallSite.t -> t
val pp : F.formatter -> t -> unit val pp : F.formatter -> t -> unit

@ -128,10 +128,10 @@ module SinkKind = struct
(* taint the nth parameter (0-indexed) *) (* taint the nth parameter (0-indexed) *)
let taint_nth n kind = let taint_nth n kind =
[kind, n] Some (kind, IntSet.singleton n)
let taint_all actuals kind = let taint_all actuals kind =
List.mapi ~f:(fun actual_num _ -> kind, actual_num) actuals Some (kind, IntSet.of_list (List.mapi ~f:(fun actual_num _ -> actual_num) actuals))
(* return Some(sink kind) if [procedure_name] is in the list of externally specified sinks *) (* return Some(sink kind) if [procedure_name] is in the list of externally specified sinks *)
let get_external_sink pname actuals = let get_external_sink pname actuals =
@ -143,10 +143,10 @@ module SinkKind = struct
let kind = of_string kind in let kind = of_string kind in
try try
let n = int_of_string index in let n = int_of_string index in
Some (taint_nth n kind) taint_nth n kind
with Failure _ -> with Failure _ ->
(* couldn't parse the index, just taint everything *) (* couldn't parse the index, just taint everything *)
Some (taint_all actuals kind) taint_all actuals kind
else else
None) None)
external_sinks external_sinks
@ -154,7 +154,7 @@ module SinkKind = struct
let get pname actuals _ = let get pname actuals _ =
match pname with match pname with
| Typ.Procname.ObjC_Cpp _ -> | Typ.Procname.ObjC_Cpp _ ->
Option.value (get_external_sink pname actuals) ~default:[] get_external_sink pname actuals
| Typ.Procname.C _ -> | Typ.Procname.C _ ->
begin begin
match Typ.Procname.to_string pname with match Typ.Procname.to_string pname with
@ -163,12 +163,12 @@ module SinkKind = struct
| "brk" | "calloc" | "malloc" | "realloc" | "sbrk" -> | "brk" | "calloc" | "malloc" | "realloc" | "sbrk" ->
taint_all actuals Allocation taint_all actuals Allocation
| _ -> | _ ->
Option.value (get_external_sink pname actuals) ~default:[] get_external_sink pname actuals
end end
| Typ.Procname.Block _ -> | Typ.Procname.Block _ ->
[] None
| pname when BuiltinDecl.is_declared pname -> | pname when BuiltinDecl.is_declared pname ->
[] None
| pname -> | pname ->
failwithf "Non-C++ procname %a in C++ analysis@." Typ.Procname.pp pname failwithf "Non-C++ procname %a in C++ analysis@." Typ.Procname.pp pname

@ -218,13 +218,14 @@ module SinkKind = struct
if Typ.Procname.java_is_static pname || taint_this if Typ.Procname.java_is_static pname || taint_this
then actuals, 0 then actuals, 0
else List.tl_exn actuals, 1 in else List.tl_exn actuals, 1 in
List.mapi let indexes =
~f:(fun param_num _ -> kind, param_num + offset) IntSet.of_list (List.mapi ~f:(fun param_num _ -> param_num + offset) actuals_to_taint) in
actuals_to_taint in Some (kind, indexes) in
(* taint the nth non-"this" parameter (0-indexed) *) (* taint the nth non-"this" parameter (0-indexed) *)
let taint_nth n kind = let taint_nth n kind =
let first_index = if Typ.Procname.java_is_static pname then n else n + 1 in let first_index = if Typ.Procname.java_is_static pname then n else n + 1 in
[kind, first_index] in Some (kind, IntSet.singleton first_index) in
match pname with match pname with
| Typ.Procname.Java java_pname -> | Typ.Procname.Java java_pname ->
begin begin
@ -237,17 +238,17 @@ module SinkKind = struct
| "java.nio.file.Paths", "get" -> | "java.nio.file.Paths", "get" ->
taint_all CreateFile taint_all CreateFile
| "com.facebook.infer.builtins.InferTaint", "inferSensitiveSink" -> | "com.facebook.infer.builtins.InferTaint", "inferSensitiveSink" ->
[Other, 0] taint_nth 0 Other
| class_name, method_name -> | class_name, method_name ->
let taint_matching_supertype typename _ = let taint_matching_supertype typename _ =
match Typ.Name.name typename, method_name with match Typ.Name.name typename, method_name with
| "android.app.Activity", | "android.app.Activity",
("startActivityFromChild" | "startActivityFromFragment") -> ("startActivityFromChild" | "startActivityFromFragment") ->
Some (taint_nth 1 StartComponent) taint_nth 1 StartComponent
| "android.app.Activity", "startIntentSenderForResult" -> | "android.app.Activity", "startIntentSenderForResult" ->
Some (taint_nth 2 StartComponent) taint_nth 2 StartComponent
| "android.app.Activity", "startIntentSenderFromChild" -> | "android.app.Activity", "startIntentSenderFromChild" ->
Some (taint_nth 3 StartComponent) taint_nth 3 StartComponent
| "android.content.Context", | "android.content.Context",
("bindService" | ("bindService" |
"sendBroadcast" | "sendBroadcast" |
@ -265,9 +266,9 @@ module SinkKind = struct
"startNextMatchingActivity" | "startNextMatchingActivity" |
"startService" | "startService" |
"stopService") -> "stopService") ->
Some (taint_nth 0 StartComponent) taint_nth 0 StartComponent
| "android.content.Context", "startIntentSender" -> | "android.content.Context", "startIntentSender" ->
Some (taint_nth 1 StartComponent) taint_nth 1 StartComponent
| "android.content.Intent", | "android.content.Intent",
("parseUri" | ("parseUri" |
"getIntent" | "getIntent" |
@ -278,9 +279,9 @@ module SinkKind = struct
"setDataAndType" | "setDataAndType" |
"setDataAndTypeAndNormalize" | "setDataAndTypeAndNormalize" |
"setPackage") -> "setPackage") ->
Some (taint_nth 0 CreateIntent) taint_nth 0 CreateIntent
| "android.content.Intent", "setClassName" -> | "android.content.Intent", "setClassName" ->
Some (taint_all CreateIntent) taint_all CreateIntent
| "android.webkit.WebView", | "android.webkit.WebView",
("evaluateJavascript" | ("evaluateJavascript" |
"loadData" | "loadData" |
@ -288,7 +289,7 @@ module SinkKind = struct
"loadUrl" | "loadUrl" |
"postUrl" | "postUrl" |
"postWebMessage") -> "postWebMessage") ->
Some (taint_all JavaScript) taint_all JavaScript
| class_name, method_name -> | class_name, method_name ->
(* check the list of externally specified sinks *) (* check the list of externally specified sinks *)
let procedure = class_name ^ "." ^ method_name in let procedure = class_name ^ "." ^ method_name in
@ -299,25 +300,19 @@ module SinkKind = struct
let kind = of_string kind in let kind = of_string kind in
try try
let n = int_of_string index in let n = int_of_string index in
Some (taint_nth n kind) taint_nth n kind
with Failure _ -> with Failure _ ->
(* couldn't parse the index, just taint everything *) (* couldn't parse the index, just taint everything *)
Some (taint_all kind) taint_all kind
else else
None) None)
external_sinks in external_sinks in
begin PatternMatch.supertype_find_map_opt
match tenv
PatternMatch.supertype_find_map_opt taint_matching_supertype
tenv (Typ.Name.Java.from_string class_name)
taint_matching_supertype
(Typ.Name.Java.from_string class_name) with
| Some sinks -> sinks
| None -> []
end
end end
| pname when BuiltinDecl.is_declared pname -> [] | pname when BuiltinDecl.is_declared pname -> None
| pname -> failwithf "Non-Java procname %a in Java analysis@." Typ.Procname.pp pname | pname -> failwithf "Non-Java procname %a in Java analysis@." Typ.Procname.pp pname
let pp fmt kind = let pp fmt kind =

@ -158,16 +158,16 @@ module Make (TaintSpecification : TaintSpec.S) = struct
List.iter ~f:report_error (TraceDomain.get_reportable_paths ~cur_site trace ~trace_of_pname) List.iter ~f:report_error (TraceDomain.get_reportable_paths ~cur_site trace ~trace_of_pname)
let add_sinks sinks actuals access_tree proc_data callee_site = let add_sink sink actuals access_tree proc_data callee_site =
(* add [sink] to the trace associated with the [formal_index]th actual *) (* add [sink] to the trace associated with the [formal_index]th actual *)
let add_sink_to_actual access_tree_acc (sink_param : TraceDomain.Sink.parameter) = let add_sink_to_actual sink_index access_tree_acc =
match List.nth_exn actuals sink_param.index with match List.nth_exn actuals sink_index with
| HilExp.AccessPath actual_ap_raw -> | HilExp.AccessPath actual_ap_raw ->
let actual_ap = AccessPath.Abstracted actual_ap_raw in let actual_ap = AccessPath.Abstracted actual_ap_raw in
begin begin
match access_path_get_node actual_ap access_tree_acc proc_data with match access_path_get_node actual_ap access_tree_acc proc_data with
| Some (actual_trace, _) -> | Some (actual_trace, _) ->
let actual_trace' = TraceDomain.add_sink sink_param.sink actual_trace in let actual_trace' = TraceDomain.add_sink sink actual_trace in
report_trace actual_trace' callee_site proc_data; report_trace actual_trace' callee_site proc_data;
TaintDomain.add_trace actual_ap actual_trace' access_tree_acc TaintDomain.add_trace actual_ap actual_trace' access_tree_acc
| None -> | None ->
@ -175,7 +175,7 @@ module Make (TaintSpecification : TaintSpec.S) = struct
end end
| _ -> | _ ->
access_tree_acc in access_tree_acc in
List.fold ~f:add_sink_to_actual ~init:access_tree sinks IntSet.fold add_sink_to_actual (TraceDomain.Sink.indexes sink) access_tree
let apply_summary let apply_summary
ret_opt ret_opt
@ -382,10 +382,11 @@ module Make (TaintSpecification : TaintSpec.S) = struct
let analyze_call astate_acc callee_pname = let analyze_call astate_acc callee_pname =
let call_site = CallSite.make callee_pname callee_loc in let call_site = CallSite.make callee_pname callee_loc in
let sinks = TraceDomain.Sink.get call_site actuals proc_data.ProcData.tenv in let sink = TraceDomain.Sink.get call_site actuals proc_data.ProcData.tenv in
let astate_with_sink = match sinks with let astate_with_sink =
| [] -> astate match sink with
| sinks -> add_sinks sinks actuals astate proc_data call_site in | Some sink -> add_sink sink actuals astate proc_data call_site
| None -> astate in
let source = TraceDomain.Source.get call_site proc_data.tenv in let source = TraceDomain.Source.get call_site proc_data.tenv in
let astate_with_source = let astate_with_source =
match source with match source with
@ -400,7 +401,7 @@ module Make (TaintSpecification : TaintSpec.S) = struct
astate_with_sink in astate_with_sink in
let astate_with_summary = let astate_with_summary =
if sinks <> [] || Option.is_some source if Option.is_some source || Option.is_some sink
then then
(* don't use a summary for a procedure that is a direct source or sink *) (* don't use a summary for a procedure that is a direct source or sink *)
astate_with_source astate_with_source

@ -32,8 +32,10 @@ module MockTrace = Trace.Make(struct
let get pname _ _ = let get pname _ _ =
if String.is_prefix ~prefix:"SINK" (Typ.Procname.to_string pname) if String.is_prefix ~prefix:"SINK" (Typ.Procname.to_string pname)
then [CallSite.make pname Location.dummy, 0] then Some (CallSite.make pname Location.dummy, IntSet.singleton 0)
else [] else None
let indexes _ = IntSet.empty
end) end)
let should_report _ _ = false let should_report _ _ = false

@ -26,7 +26,7 @@ module MockTraceElem = struct
let kind t = t let kind t = t
let make kind _ = kind let make ?indexes:_ kind _ = kind
let pp fmt = function let pp fmt = function
| Kind1 -> F.fprintf fmt "Kind1" | Kind1 -> F.fprintf fmt "Kind1"
@ -67,6 +67,8 @@ module MockSink = struct
let get _ = assert false let get _ = assert false
let indexes _ = IntSet.empty
let equal = [%compare.equal : t] let equal = [%compare.equal : t]
end end

Loading…
Cancel
Save