From f6c2bd3f6126b73285901813f4a7a0aa9b88df79 Mon Sep 17 00:00:00 2001 From: Mehdi Bouaziz Date: Tue, 18 Dec 2018 08:40:59 -0800 Subject: [PATCH] [quandary] Insecure Intent Handling Reviewed By: ngorogiannis Differential Revision: D13163873 fbshipit-source-id: 3f1417f9c --- infer/src/base/IssueType.ml | 2 ++ infer/src/base/IssueType.mli | 2 ++ infer/src/quandary/JavaTrace.ml | 31 +++++++++++++------ .../codetoanalyze/java/quandary/issues.exp | 1 + 4 files changed, 26 insertions(+), 10 deletions(-) diff --git a/infer/src/base/IssueType.ml b/infer/src/base/IssueType.ml index 68d20fcd1..6d58bc1ac 100644 --- a/infer/src/base/IssueType.ml +++ b/infer/src/base/IssueType.ml @@ -281,6 +281,8 @@ let infinite_execution_time_call = from_string ~enabled:false "INFINITE_EXECUTIO let inherently_dangerous_function = from_string "INHERENTLY_DANGEROUS_FUNCTION" +let insecure_intent_handling = from_string "INSECURE_INTENT_HANDLING" + let integer_overflow_l1 = from_string "INTEGER_OVERFLOW_L1" let integer_overflow_l2 = from_string "INTEGER_OVERFLOW_L2" diff --git a/infer/src/base/IssueType.mli b/infer/src/base/IssueType.mli index 05ca9835f..76fd9b662 100644 --- a/infer/src/base/IssueType.mli +++ b/infer/src/base/IssueType.mli @@ -187,6 +187,8 @@ val infinite_execution_time_call : t val inherently_dangerous_function : t +val insecure_intent_handling : t + val integer_overflow_l1 : t val integer_overflow_l2 : t diff --git a/infer/src/quandary/JavaTrace.ml b/infer/src/quandary/JavaTrace.ml index a6e55a448..a160ad1a7 100644 --- a/infer/src/quandary/JavaTrace.ml +++ b/infer/src/quandary/JavaTrace.ml @@ -14,6 +14,7 @@ module SourceKind = struct | DrawableResource of Pvar.t (** Drawable resource ID read from a global *) | Endpoint of (Mangled.t * Typ.desc) (** source originating from formal of an endpoint *) | Intent (** external Intent or a value read from one *) + | IntentForInsecureIntentHandling | IntentFromURI (** Intent created from a URI *) | Other (** for testing or uncategorized sources *) | PrivateData (** private user or device-specific data *) @@ -78,7 +79,9 @@ module SourceKind = struct let taint_matching_supertype typename = match (Typ.Name.name typename, method_name) with | "android.app.Activity", "getIntent" -> - Some [(Intent, return)] + Some [(Intent, return); (IntentForInsecureIntentHandling, return)] + | "android.support.v4.app.FragmentActivity", "getIntent" -> + Some [(IntentForInsecureIntentHandling, return)] | "android.content.Intent", "" when actual_has_type 2 "android.net.Uri" actuals tenv -> (* taint the [this] parameter passed to the constructor *) @@ -90,6 +93,8 @@ module SourceKind = struct | "setDataAndType" | "setDataAndTypeAndNormalize" ) ) -> Some [(IntentFromURI, return)] + | "android.content.Intent", "getData" -> + Some [(IntentForInsecureIntentHandling, return)] | "android.content.Intent", "getStringExtra" -> Some [(Intent, return)] | "android.content.SharedPreferences", "getString" -> @@ -246,7 +251,7 @@ module SourceKind = struct F.pp_print_string fmt (Pvar.to_string pvar) | Endpoint (formal_name, _) -> F.fprintf fmt "Endpoint(%s)" (Mangled.to_string formal_name) - | Intent -> + | Intent | IntentForInsecureIntentHandling -> F.pp_print_string fmt "Intent" | IntentFromURI -> F.pp_print_string fmt "IntentFromURI" @@ -277,6 +282,7 @@ module SinkKind = struct | SQLRead (** escaped read to a SQL database *) | SQLWrite (** escaped write to a SQL database *) | StartComponent (** sink that launches an Activity, Service, etc. *) + | StartComponentForInsecureIntentHandling | Other (** for testing or uncategorized sinks *) [@@deriving compare] @@ -309,6 +315,8 @@ module SinkKind = struct SQLWrite | "StartComponent" -> StartComponent + | "StartComponentForInsecureIntentHandling" -> + StartComponentForInsecureIntentHandling | _ -> Other @@ -374,6 +382,11 @@ module SinkKind = struct taint_nth 2 [StartComponent] | "android.app.Activity", "startIntentSenderFromChild" -> taint_nth 3 [StartComponent] + | ( ( "android.app.Fragment" + | "android.content.Context" + | "android.support.v4.app.Fragment" ) + , "startActivity" ) -> + taint_nth 0 [StartComponent; StartComponentForInsecureIntentHandling] | ( ( "android.app.Fragment" | "android.content.Context" | "android.support.v4.app.Fragment" ) @@ -387,7 +400,6 @@ module SinkKind = struct | "sendStickyOrderedBroadcast" | "sendStickyOrderedBroadcastAsUser" | "startActivities" - | "startActivity" | "startActivityForResult" | "startActivityIfNeeded" | "startNextMatchingActivity" @@ -485,7 +497,7 @@ module SinkKind = struct "SQLRead" | SQLWrite -> "SQLWrite" - | StartComponent -> + | StartComponent | StartComponentForInsecureIntentHandling -> "StartComponent" | Other -> "Other" ) @@ -580,6 +592,8 @@ include Trace.Make (struct | DrawableResource _, OpenDrawableResource -> (* not a security issue, but useful for debugging flows from resource IDs to inflation *) Some IssueType.quandary_taint_error + | IntentForInsecureIntentHandling, StartComponentForInsecureIntentHandling -> + Some IssueType.insecure_intent_handling | IntentFromURI, StartComponent -> (* create an intent/start a component using a (possibly user-controlled) URI. may or may not be an issue; depends on where the URI comes from *) @@ -593,11 +607,8 @@ include Trace.Make (struct | Other, _ | _, Other -> (* for testing purposes, Other matches everything *) Some IssueType.quandary_taint_error - | DrawableResource _, _ - | IntentFromURI, _ - | PrivateData, _ - | _, Logging - | _, OpenDrawableResource - | _, StartComponent -> + | (DrawableResource _ | IntentForInsecureIntentHandling | IntentFromURI | PrivateData), _ + | _, (Logging | OpenDrawableResource | StartComponent | StartComponentForInsecureIntentHandling) + -> None end) diff --git a/infer/tests/codetoanalyze/java/quandary/issues.exp b/infer/tests/codetoanalyze/java/quandary/issues.exp index 32c29ecfe..0811b7142 100644 --- a/infer/tests/codetoanalyze/java/quandary/issues.exp +++ b/infer/tests/codetoanalyze/java/quandary/issues.exp @@ -103,6 +103,7 @@ codetoanalyze/java/quandary/Intents.java, codetoanalyze.java.quandary.Intents.ex codetoanalyze/java/quandary/Intents.java, codetoanalyze.java.quandary.Intents.extraToDataBad():void, 5, UNTRUSTED_INTENT_CREATION, no_bucket, ERROR, [Return from String Intent.getStringExtra(String),Call to Intent Intent.setData(Uri) with tainted index 1] codetoanalyze/java/quandary/Intents.java, codetoanalyze.java.quandary.Intents.extraToDataBad():void, 7, QUANDARY_TAINT_ERROR, no_bucket, ERROR, [Return from Object InferTaint.inferSecretSource(),Call to Intent Intent.setData(Uri) with tainted index 1] codetoanalyze/java/quandary/Intents.java, codetoanalyze.java.quandary.Intents.extraToDataBad():void, 7, UNTRUSTED_INTENT_CREATION, no_bucket, ERROR, [Return from String Intent.getStringExtra(String),Call to Intent Intent.setData(Uri) with tainted index 1] +codetoanalyze/java/quandary/Intents.java, codetoanalyze.java.quandary.Intents.reuseIntentBad(android.app.Activity):void, 1, INSECURE_INTENT_HANDLING, no_bucket, ERROR, [Return from Intent Activity.getIntent(),Call to void Activity.startActivity(Intent) with tainted index 1] codetoanalyze/java/quandary/Intents.java, codetoanalyze.java.quandary.Intents.startWithUri1Bad(android.net.Uri):void, 1, CREATE_INTENT_FROM_URI, no_bucket, ERROR, [Return from Intent.(String,Uri),Call to void Activity.startActivity(Intent) with tainted index 1] codetoanalyze/java/quandary/Intents.java, codetoanalyze.java.quandary.Intents.startWithUri2Bad(android.net.Uri):void, 1, CREATE_INTENT_FROM_URI, no_bucket, ERROR, [Return from Intent.(String,Uri,Context,Class),Call to void Activity.startActivity(Intent) with tainted index 1] codetoanalyze/java/quandary/Intents.java, codetoanalyze.java.quandary.Intents.subclassCallBad(codetoanalyze.java.quandary.IntentSubclass,codetoanalyze.java.quandary.ContextSubclass):void, 3, QUANDARY_TAINT_ERROR, no_bucket, ERROR, [Return from Object InferTaint.inferSecretSource(),Call to void Context.startActivity(Intent) with tainted index 1]