Skip to content

Commit 2a50cf8

Browse files
haby0haby0
committed
Fix
1 parent d8f5f69 commit 2a50cf8

File tree

9 files changed

+447
-48
lines changed

9 files changed

+447
-48
lines changed

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

Lines changed: 131 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,63 +1,180 @@
1+
import java.lang.reflect.Method;
2+
import java.util.HashMap;
13
import java.util.HashSet;
4+
import java.util.List;
5+
import java.util.Map;
26
import javax.servlet.http.HttpServletRequest;
37
import org.springframework.stereotype.Controller;
8+
import org.springframework.util.StringUtils;
49
import org.springframework.web.bind.annotation.GetMapping;
5-
import org.springframework.web.bind.annotation.ResponseBody;
10+
import org.springframework.web.bind.annotation.PathVariable;
11+
import org.springframework.web.bind.annotation.RequestBody;
12+
import org.springframework.web.bind.annotation.RequestMapping;
13+
import org.springframework.web.bind.annotation.RequestMethod;
14+
import org.springframework.web.multipart.MultipartFile;
615

716
@Controller
817
public class UnsafeReflection {
918

1019
@GetMapping(value = "uf1")
1120
public void bad1(HttpServletRequest request) {
1221
String className = request.getParameter("className");
22+
String parameterValue = request.getParameter("parameterValue");
1323
try {
14-
Class clazz = Class.forName(className); //bad
15-
} catch (ClassNotFoundException e) {
24+
Class clazz = Class.forName(className);
25+
Object object = clazz.getDeclaredConstructors()[0].newInstance(parameterValue); //bad
26+
} catch (Exception e) {
1627
e.printStackTrace();
1728
}
1829
}
1930

2031
@GetMapping(value = "uf2")
2132
public void bad2(HttpServletRequest request) {
2233
String className = request.getParameter("className");
34+
String parameterValue = request.getParameter("parameterValue");
2335
try {
2436
ClassLoader classLoader = ClassLoader.getSystemClassLoader();
25-
Class clazz = classLoader.loadClass(className); //bad
26-
} catch (ClassNotFoundException e) {
37+
Class clazz = classLoader.loadClass(className);
38+
Object object = clazz.newInstance();
39+
clazz.getDeclaredMethods()[0].invoke(object, parameterValue); //bad
40+
} catch (Exception e) {
2741
e.printStackTrace();
2842
}
2943
}
3044

45+
@RequestMapping(value = {"/service/{beanIdOrClassName}/{methodName}"}, method = {RequestMethod.POST}, consumes = {"application/json"}, produces = {"application/json"})
46+
public Object bad3(@PathVariable("beanIdOrClassName") String beanIdOrClassName, @PathVariable("methodName") String methodName, @RequestBody Map<String, Object> body) throws Exception {
47+
List<Object> rawData = null;
48+
try {
49+
rawData = (List<Object>)body.get("methodInput");
50+
} catch (Exception e) {
51+
return e;
52+
}
53+
return invokeService(beanIdOrClassName, methodName, null, rawData);
54+
}
55+
3156
@GetMapping(value = "uf3")
3257
public void good1(HttpServletRequest request) throws Exception {
3358
HashSet<String> hashSet = new HashSet<>();
3459
hashSet.add("com.example.test1");
3560
hashSet.add("com.example.test2");
3661
String className = request.getParameter("className");
37-
if (hashSet.contains(className)){ //good
62+
String parameterValue = request.getParameter("parameterValue");
63+
if (!hashSet.contains(className)){
3864
throw new Exception("Class not valid: " + className);
3965
}
40-
ClassLoader classLoader = ClassLoader.getSystemClassLoader();
41-
Class clazz = classLoader.loadClass(className);
66+
try {
67+
Class clazz = Class.forName(className);
68+
Object object = clazz.getDeclaredConstructors()[0].newInstance(parameterValue); //good
69+
} catch (Exception e) {
70+
e.printStackTrace();
71+
}
4272
}
4373

4474
@GetMapping(value = "uf4")
4575
public void good2(HttpServletRequest request) throws Exception {
4676
String className = request.getParameter("className");
47-
if (!"com.example.test1".equals(className)){ //good
77+
String parameterValue = request.getParameter("parameterValue");
78+
if (!"com.example.test1".equals(className)){
4879
throw new Exception("Class not valid: " + className);
4980
}
50-
ClassLoader classLoader = ClassLoader.getSystemClassLoader();
51-
Class clazz = classLoader.loadClass(className);
81+
try {
82+
Class clazz = Class.forName(className);
83+
Object object = clazz.getDeclaredConstructors()[0].newInstance(parameterValue); //good
84+
} catch (Exception e) {
85+
e.printStackTrace();
86+
}
5287
}
5388

5489
@GetMapping(value = "uf5")
5590
public void good3(HttpServletRequest request) throws Exception {
5691
String className = request.getParameter("className");
92+
String parameterValue = request.getParameter("parameterValue");
5793
if (!className.equals("com.example.test1")){ //good
5894
throw new Exception("Class not valid: " + className);
5995
}
60-
ClassLoader classLoader = ClassLoader.getSystemClassLoader();
61-
Class clazz = classLoader.loadClass(className);
96+
try {
97+
Class clazz = Class.forName(className);
98+
Object object = clazz.getDeclaredConstructors()[0].newInstance(parameterValue); //good
99+
} catch (Exception e) {
100+
e.printStackTrace();
101+
}
102+
}
103+
104+
private Object invokeService(String beanIdOrClassName, String methodName, MultipartFile[] files, List<Object> data) throws Exception {
105+
BeanFactory beanFactory = new BeanFactory();
106+
try {
107+
Object bean = null;
108+
String beanName = null;
109+
Class<?> beanClass = null;
110+
try {
111+
beanClass = Class.forName(beanIdOrClassName);
112+
beanName = StringUtils.uncapitalize(beanClass.getSimpleName());
113+
} catch (ClassNotFoundException classNotFoundException) {
114+
beanName = beanIdOrClassName;
115+
}
116+
try {
117+
bean = beanFactory.getBean(beanName);
118+
} catch (BeansException beansException) {
119+
bean = beanFactory.getBean(beanClass);
120+
}
121+
byte b;
122+
int i;
123+
Method[] arrayOfMethod;
124+
for (i = (arrayOfMethod = bean.getClass().getMethods()).length, b = 0; b < i; ) {
125+
Method method = arrayOfMethod[b];
126+
if (!method.getName().equals(methodName)) {
127+
b++;
128+
continue;
129+
}
130+
ProxygenSerializer serializer = new ProxygenSerializer();
131+
Object[] methodInput = serializer.deserializeMethodInput(data, files, method);
132+
Object result = method.invoke(bean, methodInput);
133+
Map<String, Object> map = new HashMap<>();
134+
map.put("result", serializer.serialize(result));
135+
return map;
136+
}
137+
} catch (Exception e) {
138+
return e;
139+
}
140+
return null;
141+
}
142+
}
143+
144+
class BeansException extends Exception {
145+
146+
}
147+
148+
class BeanFactory {
149+
150+
private static HashMap<String, Object> classNameMap = new HashMap<>();
151+
152+
private static HashMap<Class<?>, Object> classMap = new HashMap<>();;
153+
154+
static {
155+
classNameMap.put("xxxx", Runtime.getRuntime());
156+
classMap.put(Runtime.class, Runtime.getRuntime());
157+
}
158+
159+
public Object getBean(String className) throws BeansException {
160+
if (classNameMap.get(className) == null) {
161+
throw new BeansException();
162+
}
163+
return classNameMap.get(className);
164+
}
165+
166+
public Object getBean(Class<?> clzz) {
167+
return classMap.get(clzz);
168+
}
169+
}
170+
171+
class ProxygenSerializer {
172+
173+
public Object[] deserializeMethodInput(List<Object> data, MultipartFile[] files, Method method) {
174+
return null;
175+
}
176+
177+
public String serialize(Object result) {
178+
return null;
62179
}
63-
}
180+
}

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

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,8 @@
33

44
<overview>
55
<p>
6-
Allowing users to freely select a class to load can result in invocation of unexpected dangerous code.
7-
Dynamically loaded classes could contain dangerous code executed by a constructor or
8-
static class initializer, which means a vulnerability can rairse even without invoking methods
9-
on such classes to be vulnerable to an attack.
6+
Allowing users to freely choose to call the constructor or method of the class through reflection. This means that
7+
malicious users can carefully construct requests to create instances of any class or call methods of any class.
108
</p>
119
</overview>
1210

@@ -19,9 +17,10 @@ users can only execute classes or codes that are running.
1917

2018
<example>
2119
<p>
22-
The following examples show good examples and bad examples respectively. When using <code>Class.forName(...)</code>
23-
or <code>ClassLoader.loadClass(...)</code> and not verifying user input, it is easy to cause security risks, for example:
24-
<code>bad1</code>/<code>bad2</code>. When the user input is verified by <code>contains</code> or <code>equals</code> and then
20+
The following examples are good examples and bad examples. When using <code>Class.forName(...)</code> or
21+
<code>ClassLoader.loadClass(...)</code> to get the class, then create the class object or call the method of the class.
22+
When the user is not authenticated Input, it is easy to cause security risks, for example: <code>bad1</code>/<code>bad2
23+
</code>/<code>bad3</code>. When the user input is verified by <code>contains</code> or <code>equals</code> and then
2524
the reflection operation is performed, the system security can be well controlled, for example:
2625
<code >good1</code>/<code>good2</code>/<code>good3</code>.
2726
</p>

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

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,17 @@
1111
*/
1212

1313
import java
14+
import DataFlow
1415
import UnsafeReflectionLib
16+
import semmle.code.java.dataflow.DataFlow
1517
import semmle.code.java.dataflow.FlowSources
1618
import DataFlow::PathGraph
1719

1820
private class ContainsSanitizer extends DataFlow::BarrierGuard {
1921
ContainsSanitizer() { this.(MethodAccess).getMethod().hasName("contains") }
2022

2123
override predicate checks(Expr e, boolean branch) {
22-
e = this.(MethodAccess).getArgument(0) and branch = false
24+
e = this.(MethodAccess).getArgument(0) and branch = true
2325
}
2426
}
2527

@@ -39,6 +41,45 @@ class UnsafeReflectionConfig extends TaintTracking::Configuration {
3941

4042
override predicate isSink(DataFlow::Node sink) { sink instanceof UnsafeReflectionSink }
4143

44+
override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
45+
exists(ReflectiveClassIdentifierMethodAccessCall rcimac |
46+
rcimac.getArgument(0) = pred.asExpr() and rcimac = succ.asExpr()
47+
)
48+
or
49+
exists(MethodAccess ma |
50+
(
51+
ma instanceof ReflectiveConstructorsAccess or
52+
ma instanceof ReflectiveMethodsAccess
53+
) and
54+
ma.getQualifier() = pred.asExpr() and
55+
ma = succ.asExpr()
56+
)
57+
or
58+
exists(MethodAccess ma |
59+
ma.getMethod().getReturnType() instanceof TypeObject and
60+
ma.getMethod().getAParamType() instanceof TypeClass and
61+
ma.getAnArgument() = pred.asExpr() and
62+
ma = succ.asExpr()
63+
)
64+
or
65+
exists(MethodAccess ma |
66+
ma.getMethod().hasName("getClass") and
67+
ma.getMethod().getDeclaringType().hasQualifiedName("java.lang", "Object") and
68+
ma.getQualifier() = pred.asExpr() and
69+
ma = succ.asExpr()
70+
)
71+
or
72+
exists(MethodAccess ma, Method m, int i, Expr arg |
73+
m = ma.getMethod() and arg = ma.getArgument(i)
74+
|
75+
m.getReturnType() instanceof JacksonTypeDescriptorType and
76+
m.getName().toLowerCase().regexpMatch("resolve|load|class|type") and
77+
arg.getType() instanceof TypeString and
78+
arg = pred.asExpr() and
79+
ma = succ.asExpr()
80+
)
81+
}
82+
4283
override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
4384
guard instanceof ContainsSanitizer or guard instanceof EqualsSanitizer
4485
}

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

Lines changed: 93 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,103 @@
11
import java
2+
import DataFlow
23
import semmle.code.java.Reflection
34
import semmle.code.java.dataflow.DataFlow
5+
import semmle.code.java.dataflow.DataFlow3
6+
import semmle.code.java.dataflow.FlowSources
7+
import semmle.code.java.dataflow.TaintTracking
8+
import semmle.code.java.dataflow.TaintTracking2
9+
10+
/** Type descriptors in Jackson libraries. */
11+
class JacksonTypeDescriptorType extends RefType {
12+
JacksonTypeDescriptorType() {
13+
this instanceof TypeClass or
14+
hasQualifiedName("com.fasterxml.jackson.databind", "JavaType") or
15+
hasQualifiedName("com.fasterxml.jackson.core.type", "TypeReference")
16+
}
17+
}
18+
19+
/**
20+
* A call to a Java standard library method which constructs or returns a `Class<T>` from a `String`.
21+
* e.g `Class.forName(...)` or `ClassLoader.loadClass(...)`
22+
*/
23+
class ReflectiveClassIdentifierMethodAccessCall extends MethodAccess {
24+
ReflectiveClassIdentifierMethodAccessCall() {
25+
this instanceof ReflectiveClassIdentifierMethodAccess
26+
}
27+
}
428

529
/**
6-
* Unsafe reflection sink,
7-
* e.g `Class.forName(...)` or `ClassLoader.loadClass(...)`.
30+
* Unsafe reflection sink.
31+
* e.g `Constructor.newInstance(...)` or `Method.invoke(...)` or `Class.newInstance()`.
832
*/
933
class UnsafeReflectionSink extends DataFlow::ExprNode {
1034
UnsafeReflectionSink() {
11-
exists(ReflectiveClassIdentifierMethodAccess rcima | rcima.getArgument(0) = this.getExpr())
35+
exists(MethodAccess ma |
36+
(
37+
ma.getMethod().hasQualifiedName("java.lang.reflect", "Constructor<>", "newInstance")
38+
or
39+
ma.getMethod().hasQualifiedName("java.lang.reflect", "Method", "invoke")
40+
) and
41+
ma.getQualifier() = this.asExpr() and
42+
exists(ReflectionArgsConfig rac | rac.hasFlowToExpr(ma.getAnArgument()))
43+
)
44+
}
45+
}
46+
47+
/** Taint-tracking configuration tracing flow from remote sources to specifying the initialization parameters to the constructor or method. */
48+
class ReflectionArgsConfig extends TaintTracking2::Configuration {
49+
ReflectionArgsConfig() { this = "ReflectionArgsConfig" }
50+
51+
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
52+
53+
override predicate isSink(DataFlow::Node sink) {
54+
exists(NewInstance ni | ni.getAnArgument() = sink.asExpr())
55+
or
56+
exists(MethodAccess ma, ReflectionInvokeObjectConfig rioc |
57+
ma.getMethod().hasQualifiedName("java.lang.reflect", "Method", "invoke") and
58+
ma.getArgument(1) = sink.asExpr() and
59+
rioc.hasFlow(_, DataFlow::exprNode(ma.getArgument(0)))
60+
)
61+
}
62+
63+
override predicate isAdditionalTaintStep(Node pred, Node succ) {
64+
exists(MethodAccess ma |
65+
ma.getMethod().getReturnType().hasName("Object[]") and
66+
ma.getAnArgument() = pred.asExpr() and
67+
ma = succ.asExpr()
68+
)
69+
}
70+
}
71+
72+
/** A data flow configuration tracing flow from the class object associated with the class to specifying the initialization parameters. */
73+
class ReflectionInvokeObjectConfig extends DataFlow3::Configuration {
74+
ReflectionInvokeObjectConfig() { this = "ReflectionInvokeObjectConfig" }
75+
76+
override predicate isSource(DataFlow::Node source) {
77+
exists(ReflectiveClassIdentifierMethodAccessCall rma | rma = source.asExpr())
78+
}
79+
80+
override predicate isSink(DataFlow::Node sink) {
81+
exists(MethodAccess ma |
82+
ma.getMethod().hasQualifiedName("java.lang.reflect", "Method", "invoke") and
83+
ma.getArgument(0) = sink.asExpr()
84+
)
85+
}
86+
87+
override predicate isAdditionalFlowStep(Node pred, Node succ) {
88+
exists(NewInstance ni |
89+
ni.getQualifier() = pred.asExpr() and
90+
ni = succ.asExpr()
91+
)
92+
or
93+
exists(MethodAccess ma |
94+
ma.getMethod().getReturnType() instanceof TypeObject and
95+
(
96+
ma.getMethod().getAParamType() instanceof TypeString or
97+
ma.getMethod().getAParamType() instanceof TypeClass
98+
) and
99+
ma.getAnArgument() = pred.asExpr() and
100+
ma = succ.asExpr()
101+
)
12102
}
13103
}

0 commit comments

Comments
 (0)