Skip to content

Commit eda3d86

Browse files
haby0haby0
committed
Model written using smowton
1 parent 00f13e1 commit eda3d86

File tree

4 files changed

+116
-86
lines changed

4 files changed

+116
-86
lines changed

‎java/ql/src/experimental/Security/CWE/CWE-470/UnsafeReflection.ql

Lines changed: 44 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,12 @@ class UnsafeReflectionConfig extends TaintTracking::Configuration {
4242
override predicate isSink(DataFlow::Node sink) { sink instanceof UnsafeReflectionSink }
4343

4444
override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
45-
exists(ReflectiveClassIdentifierMethodAccessCall rcimac |
45+
// Argument -> return of Class.forName, ClassLoader.loadClass
46+
exists(ReflectiveClassIdentifierMethodAccess rcimac |
4647
rcimac.getArgument(0) = pred.asExpr() and rcimac = succ.asExpr()
4748
)
4849
or
50+
// Qualifier -> return of Class.getDeclaredConstructors/Methods and similar
4951
exists(MethodAccess ma |
5052
(
5153
ma instanceof ReflectiveConstructorsAccess or
@@ -55,31 +57,42 @@ class UnsafeReflectionConfig extends TaintTracking::Configuration {
5557
ma = succ.asExpr()
5658
)
5759
or
58-
exists(
59-
MethodAccess ma // Object.getClass()
60-
|
60+
// Qualifier -> return of Object.getClass
61+
exists(MethodAccess ma |
6162
ma.getMethod().hasName("getClass") and
6263
ma.getMethod().getDeclaringType().hasQualifiedName("java.lang", "Object") and
6364
ma.getQualifier() = pred.asExpr() and
6465
ma = succ.asExpr()
6566
)
6667
or
67-
exists(MethodAccess ma, Method m, int i, Expr arg |
68-
m = ma.getMethod() and arg = ma.getArgument(i)
69-
|
70-
m.getReturnType() instanceof TypeClass and
71-
arg.getType() instanceof TypeString and
72-
arg = pred.asExpr() and
73-
ma = succ.asExpr()
68+
// Argument -> return of methods that look like Class.forName
69+
looksLikeResolveClassStep(pred, succ)
70+
or
71+
// Argument -> return of methods that look like `Object getInstance(Class c)`
72+
looksLikeInstantiateClassStep(pred, succ)
73+
or
74+
// Argument -> return of BeanFactory.getBean
75+
exists(MethodAccess ma, Method getBean, Expr argument |
76+
getBean.hasQualifiedName("org.springframework.beans.factory", "BeanFactory", "getBean") and
77+
(
78+
ma.getMethod().overrides(getBean)
79+
or
80+
ma.getMethod() = getBean
81+
) and
82+
argument = ma.getAnArgument() and
83+
(
84+
argument.getType() instanceof TypeString
85+
or
86+
argument.getType() instanceof TypeClass
87+
) and
88+
pred.asExpr() = argument and
89+
succ.asExpr() = ma
7490
)
7591
or
76-
exists(MethodAccess ma, Method m, int i, Expr arg |
77-
m = ma.getMethod() and arg = ma.getArgument(i)
78-
|
79-
m.getReturnType() instanceof TypeObject and
80-
arg.getType() instanceof TypeClass and
81-
arg = pred.asExpr() and
82-
ma = succ.asExpr()
92+
// Qualifier -> return of Constructor.newInstance, Class.newInstance
93+
exists(NewInstance ni |
94+
ni.getQualifier() = pred.asExpr() and
95+
ni = succ.asExpr()
8396
)
8497
}
8598

@@ -88,6 +101,17 @@ class UnsafeReflectionConfig extends TaintTracking::Configuration {
88101
}
89102
}
90103

91-
from DataFlow::PathNode source, DataFlow::PathNode sink, UnsafeReflectionConfig conf
92-
where conf.hasFlowPath(source, sink)
104+
private Expr getAMethodArgument(MethodAccess reflectiveCall) {
105+
result = reflectiveCall.(NewInstance).getAnArgument()
106+
or
107+
result = reflectiveCall.(MethodInvokeCall).getAnArgument()
108+
}
109+
110+
from
111+
DataFlow::PathNode source, DataFlow::PathNode sink, UnsafeReflectionConfig conf,
112+
MethodAccess reflectiveCall
113+
where
114+
conf.hasFlowPath(source, sink) and
115+
sink.getNode().asExpr() = reflectiveCall.getQualifier() and
116+
conf.hasFlowToExpr(getAMethodArgument(reflectiveCall))
93117
select sink.getNode(), source, sink, "Unsafe reflection of $@.", source.getNode(), "user input"

‎java/ql/src/experimental/Security/CWE/CWE-470/UnsafeReflectionLib.qll

Lines changed: 39 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -1,83 +1,62 @@
11
import java
22
import DataFlow
33
import semmle.code.java.Reflection
4-
import semmle.code.java.dataflow.DataFlow3
54
import semmle.code.java.dataflow.FlowSources
6-
import semmle.code.java.dataflow.TaintTracking2
75

86
/**
9-
* A call to a Java standard library method which constructs or returns a `Class<T>` from a `String`.
10-
* e.g `Class.forName(...)` or `ClassLoader.loadClass(...)`
7+
* A call to `java.lang.reflect.Method.invoke`.
118
*/
12-
class ReflectiveClassIdentifierMethodAccessCall extends MethodAccess {
13-
ReflectiveClassIdentifierMethodAccessCall() {
14-
this instanceof ReflectiveClassIdentifierMethodAccess
15-
}
9+
class MethodInvokeCall extends MethodAccess {
10+
MethodInvokeCall() { this.getMethod().hasQualifiedName("java.lang.reflect", "Method", "invoke") }
1611
}
1712

1813
/**
19-
* Unsafe reflection sink.
20-
* e.g `Constructor.newInstance(...)` or `Method.invoke(...)` or `Class.newInstance()`.
14+
* Unsafe reflection sink (the qualifier or method arguments to `Constructor.newInstance(...)` or `Method.invoke(...)`)
2115
*/
2216
class UnsafeReflectionSink extends DataFlow::ExprNode {
2317
UnsafeReflectionSink() {
2418
exists(MethodAccess ma |
2519
(
26-
ma.getMethod().hasQualifiedName("java.lang.reflect", "Constructor<>", "newInstance")
27-
or
28-
ma.getMethod().hasQualifiedName("java.lang.reflect", "Method", "invoke")
20+
ma.getMethod().hasQualifiedName("java.lang.reflect", "Constructor<>", "newInstance") or
21+
ma instanceof MethodInvokeCall
2922
) and
30-
ma.getQualifier() = this.asExpr() and
31-
exists(ReflectionArgsConfig rac | rac.hasFlowToExpr(ma.getAnArgument()))
23+
this.asExpr() = [ma.getQualifier(), ma.getAnArgument()]
3224
)
3325
}
3426
}
3527

36-
/** Taint-tracking configuration tracing flow from remote sources to specifying the initialization parameters to the constructor or method. */
37-
class ReflectionArgsConfig extends TaintTracking2::Configuration {
38-
ReflectionArgsConfig() { this = "ReflectionArgsConfig" }
39-
40-
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
41-
42-
override predicate isSink(DataFlow::Node sink) {
43-
exists(NewInstance ni | ni.getAnArgument() = sink.asExpr())
44-
or
45-
exists(MethodAccess ma |
46-
ma.getMethod().hasQualifiedName("java.lang.reflect", "Method", "invoke") and
47-
ma.getArgument(1) = sink.asExpr() and
48-
exists(ReflectionInvokeObjectConfig rioc | rioc.hasFlowToExpr(ma.getArgument(0)))
49-
)
50-
}
28+
/**
29+
* Holds if `fromNode` to `toNode` is a dataflow step that looks like resolving a class.
30+
* A method probably resolves a class if it takes a string, returns a Class
31+
* and its name contains "resolve", "load", etc.
32+
*/
33+
predicate looksLikeResolveClassStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
34+
exists(MethodAccess ma, Method m, int i, Expr arg |
35+
m = ma.getMethod() and arg = ma.getArgument(i)
36+
|
37+
m.getReturnType() instanceof TypeClass and
38+
m.getName().toLowerCase().regexpMatch("resolve|load|class|type") and
39+
arg.getType() instanceof TypeString and
40+
arg = fromNode.asExpr() and
41+
ma = toNode.asExpr()
42+
)
5143
}
5244

53-
/** A data flow configuration tracing flow from the class object associated with the class to specifying the initialization parameters. */
54-
class ReflectionInvokeObjectConfig extends DataFlow3::Configuration {
55-
ReflectionInvokeObjectConfig() { this = "ReflectionInvokeObjectConfig" }
56-
57-
override predicate isSource(DataFlow::Node source) {
58-
exists(ReflectiveClassIdentifierMethodAccessCall rma | rma = source.asExpr())
59-
}
60-
61-
override predicate isSink(DataFlow::Node sink) {
62-
exists(MethodAccess ma |
63-
ma.getMethod().hasQualifiedName("java.lang.reflect", "Method", "invoke") and
64-
ma.getArgument(0) = sink.asExpr()
65-
)
66-
}
67-
68-
override predicate isAdditionalFlowStep(Node pred, Node succ) {
69-
exists(NewInstance ni |
70-
ni.getQualifier() = pred.asExpr() and
71-
ni = succ.asExpr()
72-
)
73-
or
74-
exists(MethodAccess ma, Method m, int i, Expr arg |
75-
m = ma.getMethod() and arg = ma.getArgument(i)
76-
|
77-
m.getReturnType() instanceof TypeObject and
78-
arg.getType() instanceof TypeClass and
79-
arg = pred.asExpr() and
80-
ma = succ.asExpr()
81-
)
82-
}
45+
/**
46+
* Holds if `fromNode` to `toNode` is a dataflow step that looks like instantiating a class.
47+
* A method probably instantiates a class if it is external, takes a Class, returns an Object
48+
* and its name contains "instantiate" or similar terms.
49+
*/
50+
predicate looksLikeInstantiateClassStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
51+
exists(MethodAccess ma, Method m, int i, Expr arg |
52+
m = ma.getMethod() and arg = ma.getArgument(i)
53+
|
54+
m.getReturnType() instanceof TypeObject and
55+
m.getName()
56+
.toLowerCase()
57+
.regexpMatch("instantiate|instance|create|make|getbean|instantiateclass") and
58+
arg.getType() instanceof TypeClass and
59+
arg = fromNode.asExpr() and
60+
ma = toNode.asExpr()
61+
)
8362
}

‎java/ql/src/semmle/code/java/Reflection.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ private class ReflectiveClassIdentifierLiteral extends ReflectiveClassIdentifier
5959
/**
6060
* A call to a Java standard library method which constructs or returns a `Class<T>` from a `String`.
6161
*/
62-
library class ReflectiveClassIdentifierMethodAccess extends ReflectiveClassIdentifier, MethodAccess {
62+
class ReflectiveClassIdentifierMethodAccess extends ReflectiveClassIdentifier, MethodAccess {
6363
ReflectiveClassIdentifierMethodAccess() {
6464
// A call to `Class.forName(...)`, from which we can infer `T` in the returned type `Class<T>`.
6565
getCallee().getDeclaringType() instanceof TypeClass and getCallee().hasName("forName")

‎java/ql/test/experimental/query-tests/security/CWE-470/UnsafeReflection.expected

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,56 @@
11
edges
22
| UnsafeReflection.java:21:28:21:60 | getParameter(...) : String | UnsafeReflection.java:25:29:25:59 | getDeclaredConstructors(...) : Constructor[] |
33
| UnsafeReflection.java:21:28:21:60 | getParameter(...) : String | UnsafeReflection.java:25:29:25:62 | ...[...] |
4+
| UnsafeReflection.java:22:33:22:70 | getParameter(...) : String | UnsafeReflection.java:25:76:25:89 | parameterValue |
45
| UnsafeReflection.java:25:29:25:59 | getDeclaredConstructors(...) : Constructor[] | UnsafeReflection.java:25:29:25:62 | ...[...] |
56
| UnsafeReflection.java:33:28:33:60 | getParameter(...) : String | UnsafeReflection.java:39:13:39:38 | getDeclaredMethods(...) : Method[] |
67
| UnsafeReflection.java:33:28:33:60 | getParameter(...) : String | UnsafeReflection.java:39:13:39:41 | ...[...] |
8+
| UnsafeReflection.java:33:28:33:60 | getParameter(...) : String | UnsafeReflection.java:39:50:39:55 | object |
9+
| UnsafeReflection.java:34:33:34:70 | getParameter(...) : String | UnsafeReflection.java:39:58:39:71 | parameterValue |
710
| UnsafeReflection.java:39:13:39:38 | getDeclaredMethods(...) : Method[] | UnsafeReflection.java:39:13:39:41 | ...[...] |
811
| UnsafeReflection.java:46:24:46:82 | beanIdOrClassName : String | UnsafeReflection.java:53:30:53:46 | beanIdOrClassName : String |
12+
| UnsafeReflection.java:46:132:46:168 | body : Map | UnsafeReflection.java:49:37:49:40 | body : Map |
13+
| UnsafeReflection.java:49:23:49:59 | (...)... : Object | UnsafeReflection.java:53:67:53:73 | rawData : Object |
14+
| UnsafeReflection.java:49:37:49:40 | body : Map | UnsafeReflection.java:49:37:49:59 | get(...) : Object |
15+
| UnsafeReflection.java:49:37:49:59 | get(...) : Object | UnsafeReflection.java:49:23:49:59 | (...)... : Object |
916
| UnsafeReflection.java:53:30:53:46 | beanIdOrClassName : String | UnsafeReflection.java:104:34:104:57 | beanIdOrClassName : String |
10-
| UnsafeReflection.java:104:34:104:57 | beanIdOrClassName : String | UnsafeReflection.java:109:31:109:39 | beanClass : Class |
17+
| UnsafeReflection.java:53:67:53:73 | rawData : Object | UnsafeReflection.java:104:102:104:118 | data : Object |
18+
| UnsafeReflection.java:62:33:62:70 | getParameter(...) : String | UnsafeReflection.java:68:76:68:89 | parameterValue |
19+
| UnsafeReflection.java:77:33:77:70 | getParameter(...) : String | UnsafeReflection.java:83:76:83:89 | parameterValue |
20+
| UnsafeReflection.java:92:33:92:70 | getParameter(...) : String | UnsafeReflection.java:98:76:98:89 | parameterValue |
1121
| UnsafeReflection.java:104:34:104:57 | beanIdOrClassName : String | UnsafeReflection.java:119:21:119:26 | method |
12-
| UnsafeReflection.java:109:11:109:40 | getBean(...) : Object | UnsafeReflection.java:119:21:119:26 | method |
13-
| UnsafeReflection.java:109:31:109:39 | beanClass : Class | UnsafeReflection.java:109:11:109:40 | getBean(...) : Object |
22+
| UnsafeReflection.java:104:34:104:57 | beanIdOrClassName : String | UnsafeReflection.java:119:35:119:38 | bean |
23+
| UnsafeReflection.java:104:102:104:118 | data : Object | UnsafeReflection.java:119:41:119:44 | data |
1424
nodes
1525
| UnsafeReflection.java:21:28:21:60 | getParameter(...) : String | semmle.label | getParameter(...) : String |
26+
| UnsafeReflection.java:22:33:22:70 | getParameter(...) : String | semmle.label | getParameter(...) : String |
1627
| UnsafeReflection.java:25:29:25:59 | getDeclaredConstructors(...) : Constructor[] | semmle.label | getDeclaredConstructors(...) : Constructor[] |
1728
| UnsafeReflection.java:25:29:25:62 | ...[...] | semmle.label | ...[...] |
29+
| UnsafeReflection.java:25:76:25:89 | parameterValue | semmle.label | parameterValue |
1830
| UnsafeReflection.java:33:28:33:60 | getParameter(...) : String | semmle.label | getParameter(...) : String |
31+
| UnsafeReflection.java:34:33:34:70 | getParameter(...) : String | semmle.label | getParameter(...) : String |
1932
| UnsafeReflection.java:39:13:39:38 | getDeclaredMethods(...) : Method[] | semmle.label | getDeclaredMethods(...) : Method[] |
2033
| UnsafeReflection.java:39:13:39:41 | ...[...] | semmle.label | ...[...] |
34+
| UnsafeReflection.java:39:50:39:55 | object | semmle.label | object |
35+
| UnsafeReflection.java:39:58:39:71 | parameterValue | semmle.label | parameterValue |
2136
| UnsafeReflection.java:46:24:46:82 | beanIdOrClassName : String | semmle.label | beanIdOrClassName : String |
37+
| UnsafeReflection.java:46:132:46:168 | body : Map | semmle.label | body : Map |
38+
| UnsafeReflection.java:49:23:49:59 | (...)... : Object | semmle.label | (...)... : Object |
39+
| UnsafeReflection.java:49:37:49:40 | body : Map | semmle.label | body : Map |
40+
| UnsafeReflection.java:49:37:49:59 | get(...) : Object | semmle.label | get(...) : Object |
2241
| UnsafeReflection.java:53:30:53:46 | beanIdOrClassName : String | semmle.label | beanIdOrClassName : String |
42+
| UnsafeReflection.java:53:67:53:73 | rawData : Object | semmle.label | rawData : Object |
43+
| UnsafeReflection.java:62:33:62:70 | getParameter(...) : String | semmle.label | getParameter(...) : String |
44+
| UnsafeReflection.java:68:76:68:89 | parameterValue | semmle.label | parameterValue |
45+
| UnsafeReflection.java:77:33:77:70 | getParameter(...) : String | semmle.label | getParameter(...) : String |
46+
| UnsafeReflection.java:83:76:83:89 | parameterValue | semmle.label | parameterValue |
47+
| UnsafeReflection.java:92:33:92:70 | getParameter(...) : String | semmle.label | getParameter(...) : String |
48+
| UnsafeReflection.java:98:76:98:89 | parameterValue | semmle.label | parameterValue |
2349
| UnsafeReflection.java:104:34:104:57 | beanIdOrClassName : String | semmle.label | beanIdOrClassName : String |
24-
| UnsafeReflection.java:109:11:109:40 | getBean(...) : Object | semmle.label | getBean(...) : Object |
25-
| UnsafeReflection.java:109:31:109:39 | beanClass : Class | semmle.label | beanClass : Class |
50+
| UnsafeReflection.java:104:102:104:118 | data : Object | semmle.label | data : Object |
2651
| UnsafeReflection.java:119:21:119:26 | method | semmle.label | method |
52+
| UnsafeReflection.java:119:35:119:38 | bean | semmle.label | bean |
53+
| UnsafeReflection.java:119:41:119:44 | data | semmle.label | data |
2754
#select
2855
| UnsafeReflection.java:25:29:25:62 | ...[...] | UnsafeReflection.java:21:28:21:60 | getParameter(...) : String | UnsafeReflection.java:25:29:25:62 | ...[...] | Unsafe reflection of $@. | UnsafeReflection.java:21:28:21:60 | getParameter(...) | user input |
2956
| UnsafeReflection.java:39:13:39:41 | ...[...] | UnsafeReflection.java:33:28:33:60 | getParameter(...) : String | UnsafeReflection.java:39:13:39:41 | ...[...] | Unsafe reflection of $@. | UnsafeReflection.java:33:28:33:60 | getParameter(...) | user input |

0 commit comments

Comments
 (0)