[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

@@ -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)

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 *)

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

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
-          | _ ->
+          | class_name, method_name ->
-              None
+              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)

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) ->

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

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);
+  }
+
 }

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)]