From d86f7771326e6e6df1c7aa1bd1c231a7a97a5485 Mon Sep 17 00:00:00 2001 From: Sam Blackshear Date: Mon, 9 Jan 2017 09:52:25 -0800 Subject: [PATCH] [quandary] considering methods of subclasses of Activity, SharedPreferences etc. as sources/sinks too Reviewed By: mburman Differential Revision: D4388426 fbshipit-source-id: 50ec829 --- infer/src/checkers/Source.ml | 8 +- infer/src/checkers/Source.mli | 4 +- infer/src/quandary/CppTrace.ml | 2 +- infer/src/quandary/JavaTrace.ml | 92 ++++++++++--------- infer/src/quandary/TaintAnalysis.ml | 2 +- infer/src/unit/TaintTests.ml | 2 +- .../codetoanalyze/java/quandary/Intents.java | 13 +++ .../codetoanalyze/java/quandary/issues.exp | 1 + 8 files changed, 73 insertions(+), 51 deletions(-) diff --git a/infer/src/checkers/Source.ml b/infer/src/checkers/Source.ml index 83d59563d..6dd483968 100644 --- a/infer/src/checkers/Source.ml +++ b/infer/src/checkers/Source.ml @@ -21,7 +21,7 @@ module type Kind = sig val unknown : t - val get : Procname.t -> t option + val get : Procname.t -> Tenv.t -> t option val get_tainted_formals : Procdesc.t -> Tenv.t -> (Mangled.t * Typ.t * t option) list end @@ -35,7 +35,7 @@ module type S = sig val get_footprint_access_path: t -> AccessPath.t option - val get : CallSite.t -> t option + val get : CallSite.t -> Tenv.t -> t option val get_tainted_formals : Procdesc.t -> Tenv.t -> (Mangled.t * Typ.t * t option) list end @@ -80,7 +80,7 @@ module Make (Kind : Kind) = struct let kind = Footprint ap in { site; kind; } - let get site = match Kind.get (CallSite.pname site) with + let get site tenv = match Kind.get (CallSite.pname site) tenv with | Some kind -> Some (make kind site) | None -> None @@ -119,7 +119,7 @@ module Dummy = struct let make_footprint _ _ = assert false let get_footprint_access_path _ = assert false - let get _ = None + let get _ _ = None let get_tainted_formals pdesc _= IList.map (fun (name, typ) -> name, typ, None) (Procdesc.get_formals pdesc) diff --git a/infer/src/checkers/Source.mli b/infer/src/checkers/Source.mli index 20821dc11..df75342b4 100644 --- a/infer/src/checkers/Source.mli +++ b/infer/src/checkers/Source.mli @@ -19,7 +19,7 @@ module type Kind = sig val unknown : t (** return Some (kind) if the procedure is a taint source, None otherwise *) - val get : Procname.t -> t option + val get : Procname.t -> Tenv.t -> t option (** return each formal of the function paired with either Some(kind) if the formal is a taint source, or None if the formal is not a taint source *) @@ -39,7 +39,7 @@ module type S = sig val get_footprint_access_path: t -> AccessPath.t option (** return Some (source) if the call site is a taint source, None otherwise *) - val get : CallSite.t -> t option + val get : CallSite.t -> Tenv.t -> t option (** return each formal of the function paired with either Some(source) if the formal is a taint source, or None if the formal is not a taint source *) diff --git a/infer/src/quandary/CppTrace.ml b/infer/src/quandary/CppTrace.ml index 02dfdad6a..327074e65 100644 --- a/infer/src/quandary/CppTrace.ml +++ b/infer/src/quandary/CppTrace.ml @@ -22,7 +22,7 @@ module Kind = struct let unknown = Unknown - let get = function + let get pname _ = match pname with | (Procname.ObjC_Cpp cpp_pname) as pname -> begin match Procname.objc_cpp_get_class_name cpp_pname, Procname.get_method pname with diff --git a/infer/src/quandary/JavaTrace.ml b/infer/src/quandary/JavaTrace.ml index bf5049552..7c3bef7f8 100644 --- a/infer/src/quandary/JavaTrace.ml +++ b/infer/src/quandary/JavaTrace.ml @@ -22,14 +22,10 @@ module SourceKind = struct let unknown = Unknown - let get = function + let get pname tenv = match pname with | Procname.Java pname -> begin match Procname.java_get_class_name pname, Procname.java_get_method pname with - | "android.content.Intent", "getStringExtra" -> - Some Intent - | "android.content.SharedPreferences", "getString" -> - Some PrivateData | "android.location.Location", ("getAltitude" | "getBearing" | "getLatitude" | "getLongitude" | "getSpeed") -> Some PrivateData @@ -42,8 +38,19 @@ module SourceKind = struct Some PrivateData | "com.facebook.infer.builtins.InferTaint", "inferSecretSource" -> Some Other - | _ -> - None + | class_name, method_name -> + let taint_matching_supertype typename _ = + match Typename.name typename, method_name with + | "android.content.Intent", "getStringExtra" -> + Some Intent + | "android.content.SharedPreferences", "getString" -> + Some PrivateData + | _ -> + None in + PatternMatch.supertype_find_map_opt + tenv + taint_matching_supertype + (Typename.Java.from_string class_name) end | pname when BuiltinDecl.is_declared pname -> None | pname -> failwithf "Non-Java procname %a in Java analysis@." Procname.pp pname @@ -114,41 +121,6 @@ module SinkKind = struct | Procname.Java java_pname -> begin match Procname.java_get_class_name java_pname, Procname.java_get_method java_pname with - | ("android.app.Activity" | "android.content.ContextWrapper" | "android.content.Context"), - ("bindService" | - "sendBroadcast" | - "sendBroadcastAsUser" | - "sendOrderedBroadcast" | - "sendStickyBroadcast" | - "sendStickyBroadcastAsUser" | - "sendStickyOrderedBroadcast" | - "sendStickyOrderedBroadcastAsUser" | - "startActivities" | - "startActivity" | - "startActivityForResult" | - "startActivityIfNeeded" | - "startNextMatchingActivity" | - "startService") -> - taint_nth 0 Intent ~report_reachable:true - | "android.app.Activity", ("startActivityFromChild" | "startActivityFromFragment") -> - taint_nth 1 Intent ~report_reachable:true - | "android.content.Intent", - ("fillIn" | - "makeMainSelectorActivity" | - "parseIntent" | - "parseUri" | - "replaceExtras" | - "setAction" | - "setClassName" | - "setData" | - "setDataAndNormalize" | - "setDataAndType" | - "setDataAndTypeAndNormalize" | - "setPackage" | - "setSelector" | - "setType" | - "setTypeAndNormalize") -> - taint_all Intent ~report_reachable:true | "android.util.Log", ("e" | "println" | "w" | "wtf") -> taint_all Logging ~report_reachable:true | "com.facebook.infer.builtins.InferTaint", "inferSensitiveSink" -> @@ -156,6 +128,42 @@ module SinkKind = struct | class_name, method_name -> let taint_matching_supertype typename _ = match Typename.name typename, method_name with + | "android.app.Activity", + ("startActivityFromChild" | "startActivityFromFragment") -> + Some (taint_nth 1 Intent ~report_reachable:true) + | "android.content.Context", + ("bindService" | + "sendBroadcast" | + "sendBroadcastAsUser" | + "sendOrderedBroadcast" | + "sendStickyBroadcast" | + "sendStickyBroadcastAsUser" | + "sendStickyOrderedBroadcast" | + "sendStickyOrderedBroadcastAsUser" | + "startActivities" | + "startActivity" | + "startActivityForResult" | + "startActivityIfNeeded" | + "startNextMatchingActivity" | + "startService") -> + Some (taint_nth 0 Intent ~report_reachable:true) + | "android.content.Intent", + ("fillIn" | + "makeMainSelectorActivity" | + "parseIntent" | + "parseUri" | + "replaceExtras" | + "setAction" | + "setClassName" | + "setData" | + "setDataAndNormalize" | + "setDataAndType" | + "setDataAndTypeAndNormalize" | + "setPackage" | + "setSelector" | + "setType" | + "setTypeAndNormalize") -> + Some (taint_all Intent ~report_reachable:true) | "android.webkit.WebChromeClient", ("onJsAlert" | "onJsBeforeUnload" | "onJsConfirm" | "onJsPrompt") -> Some (taint_all JavaScript ~report_reachable:true) diff --git a/infer/src/quandary/TaintAnalysis.ml b/infer/src/quandary/TaintAnalysis.ml index 3856fcf09..e27530b9c 100644 --- a/infer/src/quandary/TaintAnalysis.ml +++ b/infer/src/quandary/TaintAnalysis.ml @@ -433,7 +433,7 @@ module Make (TaintSpecification : TaintSpec.S) = struct | [] -> astate | sinks -> add_sinks sinks actuals astate proc_data call_site in - let source = TraceDomain.Source.get call_site in + let source = TraceDomain.Source.get call_site proc_data.tenv in let astate_with_source = match source, ret with | Some source, Some (ret_id, ret_typ) -> diff --git a/infer/src/unit/TaintTests.ml b/infer/src/unit/TaintTests.ml index c82c2fbe8..8a7e08bfe 100644 --- a/infer/src/unit/TaintTests.ml +++ b/infer/src/unit/TaintTests.ml @@ -18,7 +18,7 @@ module MockTrace = Trace.Make(struct let unknown = CallSite.dummy - let get pname = + let get pname _ = if String.is_prefix ~prefix:"SOURCE" (Procname.to_string pname) then Some (CallSite.make pname Location.dummy) else None diff --git a/infer/tests/codetoanalyze/java/quandary/Intents.java b/infer/tests/codetoanalyze/java/quandary/Intents.java index dbed01ca7..a1c7a38df 100644 --- a/infer/tests/codetoanalyze/java/quandary/Intents.java +++ b/infer/tests/codetoanalyze/java/quandary/Intents.java @@ -23,6 +23,12 @@ import com.facebook.infer.builtins.InferTaint; import org.xmlpull.v1.XmlPullParserException; +class IntentSubclass extends Intent { +} + +abstract class ContextSubclass extends Context { +} + public class Intents { private native int rand(); @@ -73,4 +79,11 @@ public class Intents { intent.setTypeAndNormalize(taintedString); // 15 sinks, 15 expected reports } + // make sure the rules apply to subclasses of Intent and Context too + void subclassCallBad(IntentSubclass intent, ContextSubclass context) { + String taintedString = (String) InferTaint.inferSecretSource(); + intent.setAction(taintedString); + context.startActivity(intent); + } + } diff --git a/infer/tests/codetoanalyze/java/quandary/issues.exp b/infer/tests/codetoanalyze/java/quandary/issues.exp index bd44731f6..eded6ed8a 100644 --- a/infer/tests/codetoanalyze/java/quandary/issues.exp +++ b/infer/tests/codetoanalyze/java/quandary/issues.exp @@ -83,6 +83,7 @@ codetoanalyze/java/quandary/Intents.java, void Intents.callAllIntentSinksBad(Int codetoanalyze/java/quandary/Intents.java, void Intents.callAllIntentSinksBad(Intent), 20, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),call to void Intent.setSelector(Intent)] codetoanalyze/java/quandary/Intents.java, void Intents.callAllIntentSinksBad(Intent), 21, QUANDARY_TAINT_ERROR, [return from String Intent.getStringExtra(String),call to Intent Intent.setType(String)] codetoanalyze/java/quandary/Intents.java, void Intents.callAllIntentSinksBad(Intent), 22, QUANDARY_TAINT_ERROR, [return from String Intent.getStringExtra(String),call to Intent Intent.setTypeAndNormalize(String)] +codetoanalyze/java/quandary/Intents.java, void Intents.subclassCallBad(IntentSubclass,ContextSubclass), 2, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),call to Intent Intent.setAction(String)] codetoanalyze/java/quandary/Interprocedural.java, Object Interprocedural.irrelevantPassthroughsIntraprocedural(Object), 4, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),flow through Object Interprocedural.relevantPassthrough(Object),call to void InferTaint.inferSensitiveSink(Object)] codetoanalyze/java/quandary/Interprocedural.java, Object Interprocedural.irrelevantPassthroughsSinkInterprocedural(Object), 3, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),flow through Object Interprocedural.relevantPassthrough(Object),call to Object Interprocedural.callSinkIrrelevantPassthrough(Object),flow through Object Interprocedural.relevantPassthrough(Object),call to void InferTaint.inferSensitiveSink(Object)] codetoanalyze/java/quandary/Interprocedural.java, Object Interprocedural.irrelevantPassthroughsSourceAndSinkInterprocedural(Object), 4, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),flow through Object Interprocedural.relevantPassthrough(Object),return from Object Interprocedural.returnSourceIrrelevantPassthrough(Object),flow through Object Interprocedural.relevantPassthrough(Object),call to Object Interprocedural.callSinkIrrelevantPassthrough(Object),flow through Object Interprocedural.relevantPassthrough(Object),call to void InferTaint.inferSensitiveSink(Object)]