From 919b35f50aa48af94a861eb97b3853cc37b0ac08 Mon Sep 17 00:00:00 2001 From: Sam Blackshear Date: Tue, 14 Feb 2017 14:05:35 -0800 Subject: [PATCH] [quandary] better taint propagation for Intent's Reviewed By: mburman Differential Revision: D4491864 fbshipit-source-id: 059b204 --- infer/src/quandary/ClangTaintAnalysis.ml | 2 +- infer/src/quandary/JavaTaintAnalysis.ml | 15 ++++++++++----- infer/src/quandary/JavaTrace.ml | 17 +---------------- infer/src/quandary/TaintAnalysis.ml | 6 +++++- infer/src/quandary/TaintSpec.ml | 2 +- infer/src/unit/TaintTests.ml | 2 +- .../codetoanalyze/java/quandary/Intents.java | 14 +------------- .../codetoanalyze/java/quandary/issues.exp | 15 +-------------- 8 files changed, 21 insertions(+), 52 deletions(-) diff --git a/infer/src/quandary/ClangTaintAnalysis.ml b/infer/src/quandary/ClangTaintAnalysis.ml index 8f47f15fa..8053670e2 100644 --- a/infer/src/quandary/ClangTaintAnalysis.ml +++ b/infer/src/quandary/ClangTaintAnalysis.ml @@ -23,6 +23,6 @@ include | QuandarySummary.AccessTree.Clang tree -> tree | _ -> assert false - let handle_unknown_call _ _ _ = + let handle_unknown_call _ _ _ _ = [] end) diff --git a/infer/src/quandary/JavaTaintAnalysis.ml b/infer/src/quandary/JavaTaintAnalysis.ml index ce96b5598..2aaa937b7 100644 --- a/infer/src/quandary/JavaTaintAnalysis.ml +++ b/infer/src/quandary/JavaTaintAnalysis.ml @@ -23,10 +23,15 @@ include | QuandarySummary.AccessTree.Java access_tree -> access_tree | _ -> assert false - let handle_unknown_call pname ret_typ_opt actuals = - let types_match typ class_string = match typ with - | Typ.Tptr (Tstruct typename, _) -> String.equal (Typename.name typename) class_string - | _ -> false in + let handle_unknown_call pname ret_typ_opt actuals tenv = + let types_match typ class_string tenv = match typ with + | Typ.Tptr (Tstruct original_typename, _) -> + PatternMatch.supertype_exists + tenv + (fun typename _ -> String.equal (Typename.name typename) class_string) + original_typename + | _ -> + false in match pname with | (Procname.Java java_pname) as pname -> let is_static = Procname.java_is_static pname in @@ -43,7 +48,7 @@ include begin match actuals with | (_, receiver_typ) :: _ - when not is_static && types_match receiver_typ classname -> + when not is_static && types_match receiver_typ classname tenv -> (* if the receiver and return type are the same, propagate to both. we're assuming the call is one of the common "builder-style" methods that both updates and returns the receiver *) diff --git a/infer/src/quandary/JavaTrace.ml b/infer/src/quandary/JavaTrace.ml index f00848abf..aa6c5f9e0 100644 --- a/infer/src/quandary/JavaTrace.ml +++ b/infer/src/quandary/JavaTrace.ml @@ -202,22 +202,7 @@ module SinkKind = struct Some (taint_nth 0 Intent ~report_reachable:true) | "android.content.Context", "startIntentSender" -> Some (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") -> + | "android.content.Intent", ("fillIn" | "parseIntent" | "parseUri") -> Some (taint_all Intent ~report_reachable:true) | "android.webkit.WebChromeClient", ("onJsAlert" | "onJsBeforeUnload" | "onJsConfirm" | "onJsPrompt") -> diff --git a/infer/src/quandary/TaintAnalysis.ml b/infer/src/quandary/TaintAnalysis.ml index 03fd666ec..6a0b006cd 100644 --- a/infer/src/quandary/TaintAnalysis.ml +++ b/infer/src/quandary/TaintAnalysis.ml @@ -402,7 +402,11 @@ module Make (TaintSpecification : TaintSpec.S) = struct astate_acc in let propagations = - TaintSpecification.handle_unknown_call callee_pname (Option.map ~f:snd ret) actuals in + TaintSpecification.handle_unknown_call + callee_pname + (Option.map ~f:snd ret) + actuals + proc_data.tenv in IList.fold_left handle_unknown_call_ astate propagations in let analyze_call astate_acc callee_pname = diff --git a/infer/src/quandary/TaintSpec.ml b/infer/src/quandary/TaintSpec.ml index 2d6c18e59..36c987825 100644 --- a/infer/src/quandary/TaintSpec.ml +++ b/infer/src/quandary/TaintSpec.ml @@ -23,7 +23,7 @@ module type S = sig (** return a summary for handling an unknown call at the given site with the given return type and actuals *) val handle_unknown_call : - Procname.t -> Typ.t option -> (Exp.t * Typ.t) list -> handle_unknown list + Procname.t -> Typ.t option -> (Exp.t * Typ.t) list -> Tenv.t -> handle_unknown list val to_summary_access_tree : AccessTree.t -> QuandarySummary.AccessTree.t diff --git a/infer/src/unit/TaintTests.ml b/infer/src/unit/TaintTests.ml index 49e8557e8..569b1f82e 100644 --- a/infer/src/unit/TaintTests.ml +++ b/infer/src/unit/TaintTests.ml @@ -45,7 +45,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 handle_unknown_call _ _ _ _ = [] end) module TestInterpreter = AnalyzerTester.Make (ProcCfg.Normal) (MockTaintAnalysis.TransferFunctions) diff --git a/infer/tests/codetoanalyze/java/quandary/Intents.java b/infer/tests/codetoanalyze/java/quandary/Intents.java index 3551ba6ae..5a0db8010 100644 --- a/infer/tests/codetoanalyze/java/quandary/Intents.java +++ b/infer/tests/codetoanalyze/java/quandary/Intents.java @@ -85,19 +85,7 @@ public class Intents { Intent intent = new Intent(); intent.fillIn(taintedIntent, 0); intent.makeMainSelectorActivity(taintedString, null); - intent.parseIntent(taintedResources, null, null); - intent.parseUri(taintedString, 0); - intent.replaceExtras(taintedIntent); - intent.setAction(taintedString); - intent.setClassName(taintedString, null); - intent.setData(taintedUri); - intent.setDataAndNormalize(taintedUri); - intent.setDataAndType(taintedUri, null); - intent.setDataAndTypeAndNormalize(taintedUri, null); - intent.setPackage(taintedString); - intent.setSelector(taintedIntent); - intent.setType(taintedString); - intent.setTypeAndNormalize(taintedString); // 15 sinks, 15 expected reports + intent.parseIntent(taintedResources, null, null); // 3 sinks, 3 expected results } // make sure the rules apply to subclasses of Intent and Context too diff --git a/infer/tests/codetoanalyze/java/quandary/issues.exp b/infer/tests/codetoanalyze/java/quandary/issues.exp index 21ff62548..4c4a963ff 100644 --- a/infer/tests/codetoanalyze/java/quandary/issues.exp +++ b/infer/tests/codetoanalyze/java/quandary/issues.exp @@ -78,22 +78,9 @@ codetoanalyze/java/quandary/Intents.java, void Intents.callAllActivitySinksBad(A codetoanalyze/java/quandary/Intents.java, void Intents.callAllActivitySinksBad(Activity,String), 22, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),call to ComponentName ContextWrapper.startService(Intent)] codetoanalyze/java/quandary/Intents.java, void Intents.callAllActivitySinksBad(Activity,String), 23, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),call to boolean ContextWrapper.stopService(Intent)] codetoanalyze/java/quandary/Intents.java, void Intents.callAllIntentSinksBad(Intent), 8, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),call to int Intent.fillIn(Intent,int)] -codetoanalyze/java/quandary/Intents.java, void Intents.callAllIntentSinksBad(Intent), 9, QUANDARY_TAINT_ERROR, [return from String Intent.getStringExtra(String),call to Intent Intent.makeMainSelectorActivity(String,String)] codetoanalyze/java/quandary/Intents.java, void Intents.callAllIntentSinksBad(Intent), 10, QUANDARY_TAINT_ERROR, [return from String Intent.getStringExtra(String),call to Intent Intent.parseIntent(Resources,XmlPullParser,AttributeSet)] -codetoanalyze/java/quandary/Intents.java, void Intents.callAllIntentSinksBad(Intent), 11, QUANDARY_TAINT_ERROR, [return from String Intent.getStringExtra(String),call to Intent Intent.parseUri(String,int)] -codetoanalyze/java/quandary/Intents.java, void Intents.callAllIntentSinksBad(Intent), 12, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),call to Intent Intent.replaceExtras(Intent)] -codetoanalyze/java/quandary/Intents.java, void Intents.callAllIntentSinksBad(Intent), 13, QUANDARY_TAINT_ERROR, [return from String Intent.getStringExtra(String),call to Intent Intent.setAction(String)] -codetoanalyze/java/quandary/Intents.java, void Intents.callAllIntentSinksBad(Intent), 14, QUANDARY_TAINT_ERROR, [return from String Intent.getStringExtra(String),call to Intent Intent.setClassName(String,String)] -codetoanalyze/java/quandary/Intents.java, void Intents.callAllIntentSinksBad(Intent), 15, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),call to Intent Intent.setData(Uri)] -codetoanalyze/java/quandary/Intents.java, void Intents.callAllIntentSinksBad(Intent), 16, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),call to Intent Intent.setDataAndNormalize(Uri)] -codetoanalyze/java/quandary/Intents.java, void Intents.callAllIntentSinksBad(Intent), 17, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),call to Intent Intent.setDataAndType(Uri,String)] -codetoanalyze/java/quandary/Intents.java, void Intents.callAllIntentSinksBad(Intent), 18, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),call to Intent Intent.setDataAndTypeAndNormalize(Uri,String)] -codetoanalyze/java/quandary/Intents.java, void Intents.callAllIntentSinksBad(Intent), 19, QUANDARY_TAINT_ERROR, [return from String Intent.getStringExtra(String),call to Intent Intent.setPackage(String)] -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.reuseIntentBad(Activity), 1, QUANDARY_TAINT_ERROR, [return from Intent Activity.getIntent(),call to void Activity.startActivity(Intent)] -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/Intents.java, void Intents.subclassCallBad(IntentSubclass,ContextSubclass), 3, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),call to void Context.startActivity(Intent)] codetoanalyze/java/quandary/Intents.java, void MyActivity.onActivityResult(int,int,Intent), 1, QUANDARY_TAINT_ERROR, [return from void MyActivity.onActivityResult(int,int,Intent),call to ComponentName ContextWrapper.startService(Intent)] codetoanalyze/java/quandary/Intents.java, void MyActivity.onNewIntent(Intent), 1, QUANDARY_TAINT_ERROR, [return from void MyActivity.onNewIntent(Intent),call to ComponentName ContextWrapper.startService(Intent)] 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)]