mirror of
https://github.com/github/codeql.git
synced 2025-12-24 04:36:35 +01:00
Remove checks for dynamically registered recievers
This commit is contained in:
@@ -55,85 +55,20 @@ string getASystemActionName() {
|
||||
}
|
||||
|
||||
/** An expression or XML attribute that contains the name of a system intent action. */
|
||||
class SystemActionName extends Top {
|
||||
class SystemActionName extends AndroidActionXmlElement {
|
||||
string name;
|
||||
|
||||
SystemActionName() {
|
||||
name = getASystemActionName() and
|
||||
(
|
||||
this.(CompileTimeConstantExpr).getStringValue() = "android.intent.action." + name
|
||||
or
|
||||
this.(FieldRead).getField().hasQualifiedName("android.content", "Intent", "ACTION_" + name)
|
||||
or
|
||||
this.(AndroidActionXmlElement).getActionName() = "android.intent.action." + name
|
||||
)
|
||||
this.getActionName() = "android.intent.action." + name
|
||||
}
|
||||
|
||||
/** Gets the name of the system intent that this expression or attribute represents. */
|
||||
string getName() { result = name }
|
||||
|
||||
override string toString() { result = [this.(Expr).toString(), this.(XMLAttribute).toString()] }
|
||||
}
|
||||
|
||||
/** A call to `Context.registerReceiver` */
|
||||
private class RegisterReceiverCall extends MethodAccess {
|
||||
RegisterReceiverCall() {
|
||||
this.getMethod()
|
||||
.getASourceOverriddenMethod*()
|
||||
.hasQualifiedName("android.content", "Context", "registerReceiver")
|
||||
}
|
||||
|
||||
/** Gets the `BroadcastReceiver` argument to this call. */
|
||||
Expr getReceiverArgument() { result = this.getArgument(0) }
|
||||
|
||||
/** Gets the `IntentFilter` argument to this call. */
|
||||
Expr getFilterArgument() { result = this.getArgument(1) }
|
||||
}
|
||||
|
||||
/** A configuration to detect uses of `registerReceiver` with system intent actions. */
|
||||
private class RegisterSystemActionConfig extends DataFlow::Configuration {
|
||||
RegisterSystemActionConfig() { this = "RegisterSystemActionConfig" }
|
||||
|
||||
override predicate isSource(DataFlow::Node node) { node.asExpr() instanceof SystemActionName }
|
||||
|
||||
override predicate isSink(DataFlow::Node node) {
|
||||
exists(RegisterReceiverCall ma | node.asExpr() = ma.getFilterArgument())
|
||||
}
|
||||
|
||||
override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
|
||||
exists(ConstructorCall cc |
|
||||
cc.getConstructedType().hasQualifiedName("android.content", "IntentFilter") and
|
||||
node1.asExpr() = cc.getArgument(0) and
|
||||
node2.asExpr() = cc
|
||||
)
|
||||
or
|
||||
exists(MethodAccess ma |
|
||||
ma.getMethod().hasQualifiedName("android.content", "IntentFilter", "create") and
|
||||
node1.asExpr() = ma.getArgument(0) and
|
||||
node2.asExpr() = ma
|
||||
)
|
||||
or
|
||||
exists(MethodAccess ma |
|
||||
ma.getMethod().hasQualifiedName("android.content", "IntentFilter", "addAction") and
|
||||
node1.asExpr() = ma.getArgument(0) and
|
||||
node2.(DataFlow::PostUpdateNode).getPreUpdateNode().asExpr() = ma.getQualifier()
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/** Holds if `rrc` registers a receiver `orm` to receive the system action `sa` that doesn't verify the intents it receives. */
|
||||
private predicate registeredUnverifiedSystemReceiver(
|
||||
RegisterReceiverCall rrc, UnverifiedOnReceiveMethod orm, SystemActionName sa
|
||||
) {
|
||||
exists(RegisterSystemActionConfig conf, ConstructorCall cc |
|
||||
conf.hasFlow(DataFlow::exprNode(sa), DataFlow::exprNode(rrc.getFilterArgument())) and
|
||||
cc.getConstructedType() = orm.getDeclaringType() and
|
||||
DataFlow::localExprFlow(cc, rrc.getReceiverArgument())
|
||||
)
|
||||
string getSystemActionName() { result = name }
|
||||
}
|
||||
|
||||
/** Holds if the XML element `rec` declares a receiver `orm` to receive the system action named `sa` that doesn't verify intents it receives. */
|
||||
private predicate xmlUnverifiedSystemReceiver(
|
||||
predicate unverifiedSystemReceiver(
|
||||
AndroidReceiverXmlElement rec, UnverifiedOnReceiveMethod orm, SystemActionName sa
|
||||
) {
|
||||
exists(Class ormty |
|
||||
@@ -142,9 +77,3 @@ private predicate xmlUnverifiedSystemReceiver(
|
||||
rec.getAnIntentFilterElement().getAnActionElement() = sa
|
||||
)
|
||||
}
|
||||
|
||||
/** Holds if `reg` registers (either explicitly or through XML) a receiver `orm` to receive the system action named `sa` that doesn't verify the intents it receives. */
|
||||
predicate unverifiedSystemReceiver(Top reg, Method orm, SystemActionName sa) {
|
||||
registeredUnverifiedSystemReceiver(reg, orm, sa) or
|
||||
xmlUnverifiedSystemReceiver(reg, orm, sa)
|
||||
}
|
||||
|
||||
9
java/ql/src/Security/CWE/CWE-925/AndroidManifest.xml
Normal file
9
java/ql/src/Security/CWE/CWE-925/AndroidManifest.xml
Normal file
@@ -0,0 +1,9 @@
|
||||
<manifest xmlns:android="http://schemas.android.com/apk/res/android" package="test">
|
||||
<application>
|
||||
<receiver android:name=".BootReceiverXml">
|
||||
<intent-filter>
|
||||
<action android:name="android.intent.action.BOOT_COMPLETED" />
|
||||
</intent-filter>
|
||||
</receiver>
|
||||
</application>
|
||||
</manifest>
|
||||
@@ -1,9 +1,3 @@
|
||||
// ...
|
||||
IntentFilter filter = new IntentFilter(Intent.ACTION_SHUTDOWN);
|
||||
BroadcastReceiver sReceiver = new ShutDownReceiver();
|
||||
context.registerReceiver(sReceiver, filter);
|
||||
// ...
|
||||
|
||||
public class ShutdownReceiver extends BroadcastReceiver {
|
||||
@Override
|
||||
public void onReceive(final Context context, final Intent intent) {
|
||||
|
||||
@@ -1,9 +1,3 @@
|
||||
// ...
|
||||
IntentFilter filter = new IntentFilter(Intent.ACTION_SHUTDOWN);
|
||||
BroadcastReceiver sReceiver = new ShutDownReceiver();
|
||||
context.registerReceiver(sReceiver, filter);
|
||||
// ...
|
||||
|
||||
public class ShutdownReceiver extends BroadcastReceiver {
|
||||
@Override
|
||||
public void onReceive(final Context context, final Intent intent) {
|
||||
|
||||
@@ -21,6 +21,7 @@ Otherwise, a third-party application could impersonate the system this way and c
|
||||
without checking that the received action is indeed <code>ACTION_SHUTDOWN</code>. This allows third-party applications to
|
||||
send explicit intents to this receiver to cause a denial of service.</p>
|
||||
<sample src="Bad.java" />
|
||||
<sample src="AndroidManifest.xml" />
|
||||
</example>
|
||||
|
||||
<recommendation>
|
||||
|
||||
@@ -13,7 +13,7 @@
|
||||
import java
|
||||
import semmle.code.java.security.ImproperIntentVerificationQuery
|
||||
|
||||
from Top reg, Method orm, SystemActionName sa
|
||||
from AndroidReceiverXmlElement reg, Method orm, SystemActionName sa
|
||||
where unverifiedSystemReceiver(reg, orm, sa)
|
||||
select orm, "This reciever doesn't verify intents it receives, and is registered $@ to receive $@.",
|
||||
reg, "here", sa, "the system action " + sa.getName()
|
||||
|
||||
@@ -1,31 +0,0 @@
|
||||
package test;
|
||||
import android.content.Intent;
|
||||
import android.content.IntentFilter;
|
||||
import android.content.Context;
|
||||
import android.content.BroadcastReceiver;
|
||||
|
||||
class ImproperIntentVerificationTest {
|
||||
static void doStuff(Intent intent) {}
|
||||
|
||||
class ShutdownBroadcastReceiver extends BroadcastReceiver {
|
||||
@Override
|
||||
public void onReceive(Context ctx, Intent intent) { // $hasResult
|
||||
doStuff(intent);
|
||||
}
|
||||
}
|
||||
|
||||
class ShutdownBroadcastReceiverSafe extends BroadcastReceiver {
|
||||
@Override
|
||||
public void onReceive(Context ctx, Intent intent) {
|
||||
if (!intent.getAction().equals(Intent.ACTION_SHUTDOWN)) {
|
||||
return;
|
||||
}
|
||||
doStuff(intent);
|
||||
}
|
||||
}
|
||||
|
||||
void test(Context c) {
|
||||
c.registerReceiver(new ShutdownBroadcastReceiver(), new IntentFilter(Intent.ACTION_SHUTDOWN));
|
||||
c.registerReceiver(new ShutdownBroadcastReceiverSafe(), new IntentFilter(Intent.ACTION_SHUTDOWN));
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user