Skip to content

Commit 320c671

Browse files
Adress reveiw comments - make use of existing ql libraries
1 parent 9d048e7 commit 320c671

File tree

2 files changed

+12
-21
lines changed

2 files changed

+12
-21
lines changed

java/ql/lib/semmle/code/java/security/ImproperIntentVerificationQuery.qll

Lines changed: 11 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,12 @@
22

33
import java
44
import semmle.code.java.dataflow.DataFlow
5+
import semmle.code.xml.AndroidManifest
6+
import semmle.code.java.frameworks.android.Intent
57

68
/** An `onReceive` method of a `BroadcastReceiver` */
79
private class OnReceiveMethod extends Method {
8-
OnReceiveMethod() {
9-
this.getASourceOverriddenMethod*()
10-
.hasQualifiedName("android.content", "BroadcastReceiver", "onReceive")
11-
}
10+
OnReceiveMethod() { this.getASourceOverriddenMethod*() instanceof AndroidReceiveIntentMethod }
1211

1312
/** Gets the parameter of this method that holds the received `Intent`. */
1413
Parameter getIntentParameter() { result = this.getParameter(1) }
@@ -31,7 +30,7 @@ private class VerifiedIntentConfig extends DataFlow::Configuration {
3130
}
3231

3332
/** An `onReceive` method that doesn't verify the action of the intent it receives. */
34-
class UnverifiedOnReceiveMethod extends OnReceiveMethod {
33+
private class UnverifiedOnReceiveMethod extends OnReceiveMethod {
3534
UnverifiedOnReceiveMethod() {
3635
not any(VerifiedIntentConfig c).hasFlow(DataFlow::parameterNode(this.getIntentParameter()), _)
3736
}
@@ -62,21 +61,18 @@ class SystemActionName extends Top {
6261
SystemActionName() {
6362
name = getASystemActionName() and
6463
(
65-
this.(StringLiteral).getValue() = "android.intent.action." + name
64+
this.(CompileTimeConstantExpr).getStringValue() = "android.intent.action." + name
6665
or
6766
this.(FieldRead).getField().hasQualifiedName("android.content", "Intent", "ACTION_" + name)
6867
or
69-
this.(XMLAttribute).getValue() = "android.intent.action." + name
68+
this.(AndroidActionXmlElement).getActionName() = "android.intent.action." + name
7069
)
7170
}
7271

7372
/** Gets the name of the system intent that this expression or attribute represents. */
7473
string getName() { result = name }
7574

76-
override string toString() {
77-
result =
78-
[this.(StringLiteral).toString(), this.(FieldRead).toString(), this.(XMLAttribute).toString()]
79-
}
75+
override string toString() { result = [this.(Expr).toString(), this.(XMLAttribute).toString()] }
8076
}
8177

8278
/** A call to `Context.registerReceiver` */
@@ -138,17 +134,12 @@ private predicate registeredUnverifiedSystemReceiver(
138134

139135
/** Holds if the XML element `rec` declares a receiver `orm` to receive the system action named `sa` that doesn't verify intents it receives. */
140136
private predicate xmlUnverifiedSystemReceiver(
141-
XMLElement rec, UnverifiedOnReceiveMethod orm, SystemActionName sa
137+
AndroidReceiverXmlElement rec, UnverifiedOnReceiveMethod orm, SystemActionName sa
142138
) {
143-
exists(XMLElement filter, XMLElement action, Class ormty |
144-
rec.hasName("receiver") and
145-
filter.hasName("intent-filter") and
146-
action.hasName("action") and
147-
filter = rec.getAChild() and
148-
action = filter.getAChild() and
139+
exists(Class ormty |
149140
ormty = orm.getDeclaringType() and
150-
rec.getAttribute("name").getValue() = ["." + ormty.getName(), ormty.getQualifiedName()] and
151-
action.getAttribute("name") = sa
141+
rec.getComponentName() = ["." + ormty.getName(), ormty.getQualifiedName()] and
142+
rec.getAnIntentFilterElement().getAnActionElement() = sa
152143
)
153144
}
154145

java/ql/src/Security/CWE/CWE-925/ImproperIntentVerification.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,5 +15,5 @@ import semmle.code.java.security.ImproperIntentVerificationQuery
1515

1616
from Top reg, Method orm, SystemActionName sa
1717
where unverifiedSystemReceiver(reg, orm, sa)
18-
select orm, "This reciever doesn't verify intents it recieves, and is registered $@ to recieve $@.",
18+
select orm, "This reciever doesn't verify intents it receives, and is registered $@ to receive $@.",
1919
reg, "here", sa, "the system action " + sa.getName()

0 commit comments

Comments
 (0)