From 47fcbdd4b46dd83343f989f0ce093340a2ff1398 Mon Sep 17 00:00:00 2001 From: Jami Cogswell Date: Wed, 28 Sep 2022 16:36:48 -0400 Subject: [PATCH] resolve merge conflict --- java/ql/lib/semmle/code/java/Expr.qll | 17 -- .../code/java/frameworks/android/DeepLink.qll | 144 --------------- .../code/java/frameworks/android/Intent.qll | 171 ++++++++---------- .../semmle/code/java/security/DeepLink.qll | 50 +++++ .../CWE-939/AndroidDeeplinks_Experimental.ql | 62 +++++++ .../CWE-939/AndroidDeeplinks_RemoteSources.ql | 92 ---------- .../CWE-939/AndroidDeeplinks_SimpleQuery.ql | 1 + .../frameworks/android/deeplink/Test.java | 5 - .../frameworks/android/deeplink/options | 1 - .../frameworks/android/deeplink/test.expected | 0 .../frameworks/android/deeplink/test.ql | 2 - .../intent/TestStartComponentToIntent.java | 11 +- 12 files changed, 198 insertions(+), 358 deletions(-) delete mode 100644 java/ql/lib/semmle/code/java/frameworks/android/DeepLink.qll create mode 100644 java/ql/lib/semmle/code/java/security/DeepLink.qll create mode 100644 java/ql/src/Security/CWE/CWE-939/AndroidDeeplinks_Experimental.ql delete mode 100644 java/ql/src/Security/CWE/CWE-939/AndroidDeeplinks_RemoteSources.ql delete mode 100644 java/ql/test/library-tests/frameworks/android/deeplink/Test.java delete mode 100644 java/ql/test/library-tests/frameworks/android/deeplink/options delete mode 100644 java/ql/test/library-tests/frameworks/android/deeplink/test.expected delete mode 100644 java/ql/test/library-tests/frameworks/android/deeplink/test.ql diff --git a/java/ql/lib/semmle/code/java/Expr.qll b/java/ql/lib/semmle/code/java/Expr.qll index 423de505070..2c969a8d442 100644 --- a/java/ql/lib/semmle/code/java/Expr.qll +++ b/java/ql/lib/semmle/code/java/Expr.qll @@ -1316,23 +1316,6 @@ class ClassInstanceExpr extends Expr, ConstructorCall, @classinstancexpr { result = this.getAnArgument() } - // ! remove below predicate after experimentation - /** - * Gets the argument provided to the constructor of this class instance creation expression - * of the specified Type. - */ - Expr getArgumentByType(Type type) { - exists(Argument arg | - arg = this.getAnArgument() and - arg.getType() = type and - result = arg - ) - // ! e.g. use above in below code in `StartActivityIntentStep` in Intent.qll - // argType.getName().matches("Class<%>") and - // newIntent.getArgumentByType(argType).getType().(ParameterizedType).getATypeArgument() = - // getIntent.getReceiverType() and - } - /** * Gets a type argument provided to the constructor of this class instance creation expression. * diff --git a/java/ql/lib/semmle/code/java/frameworks/android/DeepLink.qll b/java/ql/lib/semmle/code/java/frameworks/android/DeepLink.qll deleted file mode 100644 index 946e2e8af4f..00000000000 --- a/java/ql/lib/semmle/code/java/frameworks/android/DeepLink.qll +++ /dev/null @@ -1,144 +0,0 @@ -/** Provides classes and predicates to reason about deep links in Android. */ - -import java -private import semmle.code.java.frameworks.android.Intent -//private import semmle.code.java.frameworks.android.AsyncTask -private import semmle.code.java.frameworks.android.Android -private import semmle.code.java.dataflow.DataFlow -//private import semmle.code.java.dataflow.DataFlow2 -private import semmle.code.java.dataflow.FlowSteps -//private import semmle.code.java.dataflow.FlowSources -//private import semmle.code.java.dataflow.ExternalFlow -//private import semmle.code.java.dataflow.TaintTracking -private import semmle.code.xml.AndroidManifest - -// ! if keeping this class, should prbly move to security folder. -// ! Remember to add 'private' annotation as needed to all new classes/predicates below. -// ! and clean-up comments, etc. in below in general... -/** - * A value-preserving step from the Intent argument of a method call that starts a component to - * a `getIntent` call or `Intent` parameter in the component that the Intent pointed to in its constructor. - */ -// ! experimental - make a DeepLink step that combine Activity, Service, Receiver, etc. -private class DeepLinkIntentStep extends AdditionalValueStep { - // DeepLinkIntentStep() { - // this instanceof StartActivityIntentStep or - // this instanceof SendBroadcastReceiverIntentStep or - // this instanceof StartServiceIntentStep - // } - override predicate step(DataFlow::Node n1, DataFlow::Node n2) { - // ! simplify below - ( - exists(StartServiceIntentStep startServiceIntentStep | startServiceIntentStep.step(n1, n2)) - or - exists(SendBroadcastReceiverIntentStep sendBroadcastIntentStep | - sendBroadcastIntentStep.step(n1, n2) - ) - or - exists(StartActivityIntentStep startActivityIntentStep | startActivityIntentStep.step(n1, n2)) - ) and - exists(AndroidComponent andComp | - andComp.getAndroidComponentXmlElement().(AndroidActivityXmlElement).hasDeepLink() and - n1.asExpr().getFile() = andComp.getFile() // ! see if better way to do this - ) - } -} - -// // ! experimentation with global flow issue - REMOVE -// /** -// * A value-preserving step from the Intent variable -// * the `Intent` Parameter in the `startActivity`. -// */ -// class IntentVariableToStartActivityStep extends AdditionalValueStep { -// override predicate step(DataFlow::Node n1, DataFlow::Node n2) { -// exists( -// MethodAccess startActivity, Variable intentTypeTest, DataFlow2::Node source, -// DataFlow2::Node sink //ClassInstanceExpr intentTypeTest | -// | -// ( -// startActivity.getMethod().overrides*(any(ContextStartActivityMethod m)) or -// startActivity.getMethod().overrides*(any(ActivityStartActivityMethod m)) -// ) and -// intentTypeTest.getType() instanceof TypeIntent and // Variable -// //intentTypeTest.getConstructedType() instanceof TypeIntent and // ClassInstanceExpr -// startActivity.getFile().getBaseName() = "MainActivity.java" and // ! REMOVE - for testing only -// //exists(StartComponentConfiguration cfg | cfg.hasFlow(source, sink)) and // GLOBAL FLOW ATTEMPT -// DataFlow::localExprFlow(intentTypeTest.getInitializer(), startActivity.getArgument(0)) and // Variable - gives 5 results - misses the 1st ProfileActivity result since no variable with that one -// //DataFlow::localExprFlow(intentTypeTest, startActivity.getArgument(0)) and // ClassInstanceExpr -// n1.asExpr() = intentTypeTest.getInitializer() and // Variable -// //n1.asExpr() = intentTypeTest and // ClassInstanceExpr -// n2.asExpr() = startActivity.getArgument(0) // ! switch to getStartActivityIntentArg(startActivity) -// ) -// } -// } -// ! rename? -// ! below works as intended when run by itself (see latest query in AndroidDeeplinks_RemoteSources.ql), -// ! but not when combined with existing flow steps (non-monotonic recursion) -// ! need to figure out how to combine, or wrap all in global flow? -class StartComponentConfiguration extends DataFlow::Configuration { - StartComponentConfiguration() { this = "StartComponentConfiguration" } - - // Override `isSource` and `isSink`. - override predicate isSource(DataFlow::Node source) { - exists(ClassInstanceExpr classInstanceExpr | - classInstanceExpr.getConstructedType() instanceof TypeIntent and - source.asExpr() = classInstanceExpr - ) - } - - override predicate isSink(DataFlow::Node sink) { - exists(MethodAccess startActivity | - // ! need to handle for all components, not just Activity - ( - startActivity.getMethod().overrides*(any(ContextStartActivityMethod m)) or - startActivity.getMethod().overrides*(any(ActivityStartActivityMethod m)) - ) and - sink.asExpr() = startActivity.getArgument(0) - ) - } - // Optionally override `isBarrier`. - // Optionally override `isAdditionalFlowStep`. - // Then, to query whether there is flow between some `source` and `sink`, - // write - // - // ```ql - // exists(MyAnalysisConfiguration cfg | cfg.hasFlow(source, sink)) - // ``` -} - -/* ********************* INTENT METHODS, E.G. parseUri, getData, getExtras, etc. ********************* */ -/* - * Below is a Draft/Test of modelling `Intent.parseUri`, `Intent.getData`, - * and `Intent.getExtras` methods - */ - -// ! Check if can use pre-existing Synthetic Field. -/** - * The method `Intent.get%Extra` or `Intent.getExtras`. - */ -class AndroidGetExtrasMethod extends Method { - AndroidGetExtrasMethod() { - this.getName().matches("get%Extra%") and - this.getDeclaringType() instanceof TypeIntent - } -} - -/** - * The method `Intent.getData` - */ -class AndroidGetDataMethod extends Method { - AndroidGetDataMethod() { - this.hasName("getData") and this.getDeclaringType() instanceof TypeIntent - } -} - -/** - * The method `Intent.parseUri` - */ -class AndroidParseUriMethod extends Method { - AndroidParseUriMethod() { - // ! Note to self: getIntent for older versions before deprecation to parseUri - (this.hasName("parseUri") or this.hasName("getIntent")) and - this.getDeclaringType() instanceof TypeIntent - } -} diff --git a/java/ql/lib/semmle/code/java/frameworks/android/Intent.qll b/java/ql/lib/semmle/code/java/frameworks/android/Intent.qll index 7bdd9dd9911..71d0df0b351 100644 --- a/java/ql/lib/semmle/code/java/frameworks/android/Intent.qll +++ b/java/ql/lib/semmle/code/java/frameworks/android/Intent.qll @@ -1,12 +1,10 @@ import java private import semmle.code.java.dataflow.DataFlow -//private import semmle.code.java.dataflow.DataFlow2 private import semmle.code.java.dataflow.ExternalFlow private import semmle.code.java.dataflow.FlowSteps -private import semmle.code.java.frameworks.android.DeepLink // ! Remember to add 'private' annotation as needed to new classes/predicates below. -// ! and clean-up comments, etc. in below in general... +// ! and clean-up comments, etc. in below in general before marking as ready-for-review. /** * The class `android.content.Intent`. */ @@ -68,31 +66,18 @@ class AndroidReceiveIntentMethod extends Method { } } -// ! confirm if peekService should be modelled since it takes an Intent as a parameter +// ! not sure if I like the name of the below class, but +// ! trying to be consistent with `AndroidReceiveIntentMethod` +// ! and `AndroidGetIntentMethod`... /** - * The method `BroadcastReceiver.peekService`. - */ -class BroadcastReceiverPeekServiceIntentMethod extends Method { - BroadcastReceiverPeekServiceIntentMethod() { - this.hasName("peekService") and this.getDeclaringType() instanceof TypeBroadcastReceiver - } -} - -// ! potentially reword the below QLDoc -/** - * A method of type Service that receives an Intent as a parameter. - * Namely, `Service.onStart`, `onStartCommand`, `onBind`, `onRebind` - * `onUnbind`, or `onTaskRemoved` + * A method of type Service that receives an Intent. + * Namely, `Service.onStart`, `onStartCommand`, `onBind`, + * `onRebind`, `onUnbind`, or `onTaskRemoved` */ class AndroidServiceIntentMethod extends Method { AndroidServiceIntentMethod() { ( - // this.hasName("onStart") or - // this.hasName("onStartCommand") or this.getName().matches("onStart%") or - // this.hasName("onBind") or - // this.hasName("onRebind") or - // this.hasName("onUnbind") or this.getName().matches("on%ind") or this.hasName("onTaskRemoved") ) and @@ -112,30 +97,22 @@ class AndroidServiceIntentMethod extends Method { } /** - * The method `Context.startActivity` or `startActivities`. + * The method `Context.startActivity`, `Context.startActivities`, + * `Activity.startActivity`,`Activity.startActivities`, + * `Activity.startActivityForResult`, `Activity.startActivityIfNeeded`, + * `Activity.startNextMatchingActivity`, `Activity.startActivityFromChild`, + * or `Activity.startActivityFromFragment`. */ -class ContextStartActivityMethod extends Method { - ContextStartActivityMethod() { - (this.hasName("startActivity") or this.hasName("startActivities")) and - this.getDeclaringType() instanceof TypeContext - } -} - -// ! maybe reword below QLDoc? -/** - * The method `Activity.startActivity`,`startActivities`, - * `startActivityForResult`, `startActivityIfNeeded`, - * `startNextMatchingActivity`, `startActivityFromChild`, - * or `startActivityFromFragment`. - */ -class ActivityStartActivityMethod extends Method { - ActivityStartActivityMethod() { +class StartActivityMethod extends Method { + StartActivityMethod() { this.getName().matches("start%Activit%") and - this.getDeclaringType() instanceof TypeActivity + ( + this.getDeclaringType() instanceof TypeContext or + this.getDeclaringType() instanceof TypeActivity + ) } } -// ! maybe reword below QLDoc? /** * The method `Context.sendBroadcast`, `sendBroadcastAsUser`, * `sendOrderedBroadcast`, `sendOrderedBroadcastAsUser`, @@ -143,30 +120,25 @@ class ActivityStartActivityMethod extends Method { * `sendStickyOrderedBroadcast`, `sendStickyOrderedBroadcastAsUser`, * or `sendBroadcastWithMultiplePermissions`. */ -class ContextSendBroadcastMethod extends Method { - ContextSendBroadcastMethod() { +class SendBroadcastMethod extends Method { + SendBroadcastMethod() { this.getName().matches("send%Broadcast%") and this.getDeclaringType() instanceof TypeContext } } -// ! update below QLDoc +// ! remove reference from below QLDoc? /** * The method `Context.startService`, `startForegroundService`, * `bindIsolatedService`, `bindService`, or `bindServiceAsUser`. + * * From https://developer.android.com/reference/android/app/Service: * "Services can be started with Context.startService() and Context.bindService()." */ -class ContextStartServiceMethod extends Method { - ContextStartServiceMethod() { +class StartServiceMethod extends Method { + StartServiceMethod() { ( - // this.hasName("startService") or - // this.hasName("startForegroundService") or - this.getName().matches("start%Service") - or - // this.hasName("bindIsolatedService") or - // this.hasName("bindService") or - // this.hasName("bindServiceAsUser") + this.getName().matches("start%Service") or this.getName().matches("bind%Service%") ) and this.getDeclaringType() instanceof TypeContext @@ -282,7 +254,7 @@ class GrantWriteUriPermissionFlag extends GrantUriPermissionFlag { GrantWriteUriPermissionFlag() { this.hasName("FLAG_GRANT_WRITE_URI_PERMISSION") } } -// ! OLD VERSION +// ! OLD VERSION - need to delete - keeping for now for reference // /** // * A value-preserving step from the Intent argument of a `startActivity` call to // * a `getIntent` call in the Activity the Intent pointed to in its constructor. @@ -301,46 +273,61 @@ class GrantWriteUriPermissionFlag extends GrantUriPermissionFlag { // ) // } // } +/* + * // ! TODO: create a parent class for the below three steps? + * // ! e.g. something like the below? + * // ! could put `getClassArgOfIntentConstructor` in parent class? + * + * // ! Also look into whether it's possible to reduce any code duplication + * // ! across the three steps in general. + * // class StartComponentIntentStep extends AdditionalValueStep { } + */ + +// The `android.Content.Intent` class has two constructors with an argument of type +// `Class`. One has the argument at position 1 and the other at position 3. +// https://developer.android.com/reference/android/content/Intent#public-constructors +private Argument getClassArgOfIntentConstructor(ClassInstanceExpr classInstanceExpr) { + classInstanceExpr.getConstructedType() instanceof TypeIntent and + if classInstanceExpr.getNumArgument() = 2 + then result = classInstanceExpr.getArgument(1) + else result = classInstanceExpr.getArgument(3) +} + /** * A value-preserving step from the Intent argument of a `startActivity` call to * a `getIntent` call in the Activity the Intent pointed to in its constructor. */ -class StartActivityIntentStep extends AdditionalValueStep { - // ! startActivityFromChild and startActivityFromFragment have Intent as argument(1), - // ! but rest have Intent as argument(0)... - // ! startActivityFromChild and startActivityFromFragment are also deprecated and - // ! may need to look into modelling androidx.fragment.app.Fragment.startActivity() as well - private Argument getStartActivityIntentArg(MethodAccess startActMethodAccess) { +private class StartActivityIntentStep extends AdditionalValueStep { + // The `startActivityFromChild` and `startActivityFromFragment` methods have + // an argument of type `Intent` at position 1, but the rest of the methods of + // type `StartActivityMethod` have an argument of type `Intent` at position 0. + private Argument getIntentArgOfStartActMethod(MethodAccess methodAccess) { + methodAccess.getMethod().overrides*(any(StartActivityMethod m)) and if - startActMethodAccess.getMethod().hasName("startActivityFromChild") or - startActMethodAccess.getMethod().hasName("startActivityFromFragment") - then result = startActMethodAccess.getArgument(1) - else result = startActMethodAccess.getArgument(0) - } - - // ! Intent has two constructors with Class parameter, only the first one with argument - // ! at position 1 was modelled before leading to lost flow. The second constructor with - // ! argument at position 3 needs to be modelled as well. - // ! See https://developer.android.com/reference/android/content/Intent#public-constructors - private Argument getIntentConstructorClassArg(ClassInstanceExpr intent) { - if intent.getNumArgument() = 2 - then result = intent.getArgument(1) - else result = intent.getArgument(3) + methodAccess.getMethod().hasName("startActivityFromChild") or + methodAccess.getMethod().hasName("startActivityFromFragment") + then result = methodAccess.getArgument(1) + else result = methodAccess.getArgument(0) } + // // The `android.Content.Intent` class has two constructors with an argument of type + // // `Class`. One has the argument at position 1 and the other at position 3. + // // https://developer.android.com/reference/android/content/Intent#public-constructors + // private Argument getClassArgOfIntentConstructor(ClassInstanceExpr classInstanceExpr) { + // classInstanceExpr.getConstructedType() instanceof TypeIntent and + // if classInstanceExpr.getNumArgument() = 2 + // then result = classInstanceExpr.getArgument(1) + // else result = classInstanceExpr.getArgument(3) + // } override predicate step(DataFlow::Node n1, DataFlow::Node n2) { exists(MethodAccess startActivity, MethodAccess getIntent, ClassInstanceExpr newIntent | - ( - // ! look for better way to handle the below - startActivity.getMethod().overrides*(any(ContextStartActivityMethod m)) or - startActivity.getMethod().overrides*(any(ActivityStartActivityMethod m)) - ) and + startActivity.getMethod().overrides*(any(StartActivityMethod m)) and getIntent.getMethod().overrides*(any(AndroidGetIntentMethod m)) and newIntent.getConstructedType() instanceof TypeIntent and - DataFlow::localExprFlow(newIntent, getStartActivityIntentArg(startActivity)) and - getIntentConstructorClassArg(newIntent).getType().(ParameterizedType).getATypeArgument() = + DataFlow::localExprFlow(newIntent, getIntentArgOfStartActMethod(startActivity)) and + getClassArgOfIntentConstructor(newIntent).getType().(ParameterizedType).getATypeArgument() = getIntent.getReceiverType() and - n1.asExpr() = getStartActivityIntentArg(startActivity) and + n1.asExpr() = getIntentArgOfStartActMethod(startActivity) and n2.asExpr() = getIntent ) } @@ -348,17 +335,17 @@ class StartActivityIntentStep extends AdditionalValueStep { /** * A value-preserving step from the Intent argument of a `sendBroadcast` call to - * the `Intent` Parameter in the `onStart` method of the BroadcastReceiver the + * the `Intent` parameter in the `onReceive` method of the BroadcastReceiver the * Intent pointed to in its constructor. */ -class SendBroadcastReceiverIntentStep extends AdditionalValueStep { +private class SendBroadcastReceiverIntentStep extends AdditionalValueStep { override predicate step(DataFlow::Node n1, DataFlow::Node n2) { exists(MethodAccess sendBroadcast, Method onReceive, ClassInstanceExpr newIntent | - sendBroadcast.getMethod().overrides*(any(ContextSendBroadcastMethod m)) and + sendBroadcast.getMethod().overrides*(any(SendBroadcastMethod m)) and onReceive.overrides*(any(AndroidReceiveIntentMethod m)) and newIntent.getConstructedType() instanceof TypeIntent and DataFlow::localExprFlow(newIntent, sendBroadcast.getArgument(0)) and - newIntent.getArgument(1).getType().(ParameterizedType).getATypeArgument() = + getClassArgOfIntentConstructor(newIntent).getType().(ParameterizedType).getATypeArgument() = onReceive.getDeclaringType() and n1.asExpr() = sendBroadcast.getArgument(0) and n2.asParameter() = onReceive.getParameter(1) @@ -366,20 +353,20 @@ class SendBroadcastReceiverIntentStep extends AdditionalValueStep { } } -// ! potentially update QLDoc (e.g. remove `onStart` reference) +// ! potentially reword QLDoc /** * A value-preserving step from the Intent argument of a `startService` call to - * the `Intent` Parameter in the `onStart` method of the Service the Intent pointed - * to in its constructor. + * the `Intent` parameter in an `AndroidServiceIntentMethod` of the Service the + * Intent pointed to in its constructor. */ -class StartServiceIntentStep extends AdditionalValueStep { +private class StartServiceIntentStep extends AdditionalValueStep { override predicate step(DataFlow::Node n1, DataFlow::Node n2) { exists(MethodAccess startService, Method serviceIntent, ClassInstanceExpr newIntent | - startService.getMethod().overrides*(any(ContextStartServiceMethod m)) and + startService.getMethod().overrides*(any(StartServiceMethod m)) and serviceIntent.overrides*(any(AndroidServiceIntentMethod m)) and newIntent.getConstructedType() instanceof TypeIntent and DataFlow::localExprFlow(newIntent, startService.getArgument(0)) and - newIntent.getArgument(1).getType().(ParameterizedType).getATypeArgument() = + getClassArgOfIntentConstructor(newIntent).getType().(ParameterizedType).getATypeArgument() = serviceIntent.getDeclaringType() and n1.asExpr() = startService.getArgument(0) and n2.asParameter() = serviceIntent.getParameter(0) diff --git a/java/ql/lib/semmle/code/java/security/DeepLink.qll b/java/ql/lib/semmle/code/java/security/DeepLink.qll new file mode 100644 index 00000000000..62d27187e84 --- /dev/null +++ b/java/ql/lib/semmle/code/java/security/DeepLink.qll @@ -0,0 +1,50 @@ +/** Provides classes and predicates to reason about deep links in Android. */ + +import java +private import semmle.code.java.frameworks.android.Intent +private import semmle.code.java.frameworks.android.Android +private import semmle.code.java.dataflow.DataFlow +private import semmle.code.java.dataflow.TaintTracking +private import semmle.code.java.dataflow.FlowSteps +private import semmle.code.xml.AndroidManifest + +// ! Experimentation file +// /** +// * A value-preserving step from the Intent argument of a method call that starts a component to +// * a `getIntent` call or `Intent` parameter in the component that the Intent pointed to in its constructor. +// */ +// // ! experimental - make a DeepLink step that combine Activity, Service, Receiver, etc. +// private class DeepLinkIntentStep extends AdditionalValueStep { +// // DeepLinkIntentStep() { +// // this instanceof StartActivityIntentStep or +// // this instanceof SendBroadcastReceiverIntentStep or +// // this instanceof StartServiceIntentStep +// // } +// override predicate step(DataFlow::Node n1, DataFlow::Node n2) { +// // ! simplify below +// ( +// exists(StartServiceIntentStep startServiceIntentStep | startServiceIntentStep.step(n1, n2)) +// or +// exists(SendBroadcastReceiverIntentStep sendBroadcastIntentStep | +// sendBroadcastIntentStep.step(n1, n2) +// ) +// or +// exists(StartActivityIntentStep startActivityIntentStep | startActivityIntentStep.step(n1, n2)) +// ) and +// exists(AndroidComponent andComp | +// andComp.getAndroidComponentXmlElement().(AndroidActivityXmlElement).hasDeepLink() and +// n1.asExpr().getFile() = andComp.getFile() // ! see if better way to do this +// ) +// } +// } +// ! experimental modelling of `parseUri` +/** + * The method `Intent.parseUri` + */ +class AndroidParseUriMethod extends Method { + AndroidParseUriMethod() { + // ! Note: getIntent for older versions before deprecation to parseUri + (this.hasName("parseUri") or this.hasName("getIntent")) and + this.getDeclaringType() instanceof TypeIntent + } +} diff --git a/java/ql/src/Security/CWE/CWE-939/AndroidDeeplinks_Experimental.ql b/java/ql/src/Security/CWE/CWE-939/AndroidDeeplinks_Experimental.ql new file mode 100644 index 00000000000..f7ebc70a165 --- /dev/null +++ b/java/ql/src/Security/CWE/CWE-939/AndroidDeeplinks_Experimental.ql @@ -0,0 +1,62 @@ +/** + * @name Android deep links + * @description Android deep links + * @problem.severity recommendation + * @security-severity 0.1 + * @id java/android/deeplinks + * @tags security + * external/cwe/cwe-939 + * @precision high + */ + +// ! REMOVE this file +// * experiment with StartActivityIntentStep +import java +import semmle.code.xml.AndroidManifest + +// import semmle.code.java.dataflow.DataFlow +// from StartServiceIntentStep startServiceIntStep, DataFlow::Node n1, DataFlow::Node n2 +// where startServiceIntStep.step(n1, n2) +// select n2, "placeholder" +// * experiment with Global Flow +// import java +// import semmle.code.java.dataflow.TaintTracking +// import semmle.code.java.frameworks.android.Intent +// import semmle.code.java.frameworks.android.Android +// import semmle.code.java.dataflow.DataFlow +// import semmle.code.java.dataflow.FlowSteps +// import semmle.code.java.dataflow.FlowSources +// import semmle.code.java.dataflow.ExternalFlow +// import semmle.code.xml.AndroidManifest +// import semmle.code.java.dataflow.TaintTracking +// class StartComponentConfiguration extends DataFlow::Configuration { +// StartComponentConfiguration() { this = "StartComponentConfiguration" } +// // Override `isSource` and `isSink`. +// override predicate isSource(DataFlow::Node source) { +// exists(ClassInstanceExpr classInstanceExpr | +// classInstanceExpr.getConstructedType() instanceof TypeIntent and +// source.asExpr() = classInstanceExpr +// ) +// } +// override predicate isSink(DataFlow::Node sink) { +// exists(MethodAccess startActivity | +// ( +// startActivity.getMethod().overrides*(any(ContextStartActivityMethod m)) or +// startActivity.getMethod().overrides*(any(ActivityStartActivityMethod m)) +// ) and +// sink.asExpr() = startActivity.getArgument(0) +// ) +// } +// } +// from +// DataFlow::Node src, DataFlow::Node sink, StartComponentConfiguration config, +// where +// config.hasFlow(src, sink) and +// sink.asExpr().getFile().getBaseName() = "MainActivity.java" // ! just for faster testing, remove when done +// select src, "This source flows to this $@.", sink, "sink" +// * simple query as placeholder +from AndroidActivityXmlElement actXmlElement +where + actXmlElement.hasDeepLink() and + not actXmlElement.getFile().(AndroidManifestXmlFile).isInBuildDirectory() +select actXmlElement, "A deeplink is used here." diff --git a/java/ql/src/Security/CWE/CWE-939/AndroidDeeplinks_RemoteSources.ql b/java/ql/src/Security/CWE/CWE-939/AndroidDeeplinks_RemoteSources.ql deleted file mode 100644 index d33cf4f3efb..00000000000 --- a/java/ql/src/Security/CWE/CWE-939/AndroidDeeplinks_RemoteSources.ql +++ /dev/null @@ -1,92 +0,0 @@ -/** - * @name Android deep links - * @description Android deep links - * @problem.severity recommendation - * @security-severity 0.1 - * @id java/android/deeplinks - * @tags security - * external/cwe/cwe-939 - * @precision high - */ - -// * experiment with StartActivityIntentStep -// import java -// import semmle.code.java.frameworks.android.DeepLink -// import semmle.code.java.dataflow.DataFlow -// from StartServiceIntentStep startServiceIntStep, DataFlow::Node n1, DataFlow::Node n2 -// where startServiceIntStep.step(n1, n2) -// select n2, "placeholder" -// * experiment with taint-tracking -// import java -// import semmle.code.java.dataflow.TaintTracking -// import semmle.code.java.frameworks.android.DeepLink -// import semmle.code.java.frameworks.android.Intent -// import semmle.code.java.frameworks.android.Android -// import semmle.code.java.dataflow.DataFlow -// import semmle.code.java.dataflow.FlowSteps -// import semmle.code.java.dataflow.FlowSources -// import semmle.code.java.dataflow.ExternalFlow -// import semmle.code.xml.AndroidManifest -// import semmle.code.java.dataflow.TaintTracking -// class MyTaintTrackingConfiguration extends TaintTracking::Configuration { -// MyTaintTrackingConfiguration() { this = "MyTaintTrackingConfiguration" } -// override predicate isSource(DataFlow::Node source) { -// // exists(AndroidActivityXmlElement andActXmlElem | -// // andActXmlElem.hasDeepLink() and -// // source.asExpr() instanceof TypeActivity -// // ) -// source instanceof RemoteFlowSource and //AndroidIntentInput -// exists(AndroidComponent andComp | -// andComp.getAndroidComponentXmlElement().(AndroidActivityXmlElement).hasDeepLink() and -// source.asExpr().getFile() = andComp.getFile() // ! ugly, see if better way to do this -// ) -// } -// override predicate isSink(DataFlow::Node sink) { -// exists(MethodAccess m | -// m.getMethod().hasName("getIntent") and -// sink.asExpr() = m -// ) -// } -// } -// from DataFlow::Node src, DataFlow::Node sink, MyTaintTrackingConfiguration config -// where config.hasFlow(src, sink) -// select src, "This environment variable constructs a URL $@.", sink, "here" -// * experiment with GLOBAL FLOW -import java -import semmle.code.java.dataflow.TaintTracking -import semmle.code.java.frameworks.android.Intent -import semmle.code.java.frameworks.android.Android -import semmle.code.java.dataflow.DataFlow -import semmle.code.java.dataflow.FlowSteps -import semmle.code.java.dataflow.FlowSources -import semmle.code.java.dataflow.ExternalFlow -import semmle.code.xml.AndroidManifest -import semmle.code.java.dataflow.TaintTracking - -class StartComponentConfiguration extends DataFlow::Configuration { - StartComponentConfiguration() { this = "StartComponentConfiguration" } - - // TODO: Override `isSource` and `isSink`. - override predicate isSource(DataFlow::Node source) { - exists(ClassInstanceExpr classInstanceExpr | - classInstanceExpr.getConstructedType() instanceof TypeIntent and - source.asExpr() = classInstanceExpr - ) - } - - override predicate isSink(DataFlow::Node sink) { - exists(MethodAccess startActivity | - ( - startActivity.getMethod().overrides*(any(ContextStartActivityMethod m)) or - startActivity.getMethod().overrides*(any(ActivityStartActivityMethod m)) - ) and - sink.asExpr() = startActivity.getArgument(0) - ) - } -} - -from DataFlow::Node src, DataFlow::Node sink, StartComponentConfiguration config -where - config.hasFlow(src, sink) and - sink.asExpr().getFile().getBaseName() = "MainActivity.java" // ! just for faster testing, remove when done -select src, "This source flows to this $@.", sink, "sink" diff --git a/java/ql/src/Security/CWE/CWE-939/AndroidDeeplinks_SimpleQuery.ql b/java/ql/src/Security/CWE/CWE-939/AndroidDeeplinks_SimpleQuery.ql index 4c6ce50cd3d..8ca8506ef90 100644 --- a/java/ql/src/Security/CWE/CWE-939/AndroidDeeplinks_SimpleQuery.ql +++ b/java/ql/src/Security/CWE/CWE-939/AndroidDeeplinks_SimpleQuery.ql @@ -14,6 +14,7 @@ import java import semmle.code.xml.AndroidManifest // simple query for testing and MRVA results +// ! REMOVE this file from AndroidActivityXmlElement actXmlElement where actXmlElement.hasDeepLink() and diff --git a/java/ql/test/library-tests/frameworks/android/deeplink/Test.java b/java/ql/test/library-tests/frameworks/android/deeplink/Test.java deleted file mode 100644 index 67736c7e8fb..00000000000 --- a/java/ql/test/library-tests/frameworks/android/deeplink/Test.java +++ /dev/null @@ -1,5 +0,0 @@ -// ! adding tests in `intent` directory instead for now -public class Test { - - -} diff --git a/java/ql/test/library-tests/frameworks/android/deeplink/options b/java/ql/test/library-tests/frameworks/android/deeplink/options deleted file mode 100644 index 33cdc1ea940..00000000000 --- a/java/ql/test/library-tests/frameworks/android/deeplink/options +++ /dev/null @@ -1 +0,0 @@ -//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/google-android-9.0.0 diff --git a/java/ql/test/library-tests/frameworks/android/deeplink/test.expected b/java/ql/test/library-tests/frameworks/android/deeplink/test.expected deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/java/ql/test/library-tests/frameworks/android/deeplink/test.ql b/java/ql/test/library-tests/frameworks/android/deeplink/test.ql deleted file mode 100644 index 5d91e4e8e26..00000000000 --- a/java/ql/test/library-tests/frameworks/android/deeplink/test.ql +++ /dev/null @@ -1,2 +0,0 @@ -import java -import TestUtilities.InlineFlowTest diff --git a/java/ql/test/library-tests/frameworks/android/intent/TestStartComponentToIntent.java b/java/ql/test/library-tests/frameworks/android/intent/TestStartComponentToIntent.java index 889b50af762..bcc192958b4 100644 --- a/java/ql/test/library-tests/frameworks/android/intent/TestStartComponentToIntent.java +++ b/java/ql/test/library-tests/frameworks/android/intent/TestStartComponentToIntent.java @@ -10,7 +10,8 @@ public class TestStartComponentToIntent { return null; } - static void sink(Object sink) {} + static void sink(Object sink) { + } public void testActivity(Context ctx) { Intent intent = new Intent(null, SomeActivity.class); @@ -26,19 +27,19 @@ public class TestStartComponentToIntent { } // ! WIP - public void testService(Context ctx) { + public void testService(Context ctx) { Intent intent = new Intent(null, SomeService.class); intent.putExtra("data", (String) source()); ctx.startService(intent); } - public void testBroadcastReceiver(Context ctx) { + public void testBroadcastReceiver(Context ctx) { Intent intent = new Intent(null, SomeBroadcastReceiver.class); intent.putExtra("data", (String) source()); ctx.sendBroadcast(intent); } - static class SomeService extends Service { + static class SomeService extends Service { public void test() { // ! WIP @@ -46,7 +47,7 @@ public class TestStartComponentToIntent { } } - static class SomeBroadcastReceiver extends BroadcastReceiver { + static class SomeBroadcastReceiver extends BroadcastReceiver { public void test() { // ! WIP