Skip to content

Commit 00f13e1

Browse files
haby0haby0
committed
Modify isAdditionalTaintStep
1 parent 291ca38 commit 00f13e1

File tree

5 files changed

+87
-168
lines changed

5 files changed

+87
-168
lines changed

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

Lines changed: 30 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -62,78 +62,42 @@ public void good2(HttpServletRequest request) throws Exception {
6262

6363
private Object invokeService(String beanIdOrClassName, String methodName, MultipartFile[] files, List<Object> data) throws Exception {
6464
BeanFactory beanFactory = new BeanFactory();
65-
try {
66-
Object bean = null;
67-
String beanName = null;
68-
Class<?> beanClass = null;
69-
try {
70-
beanClass = Class.forName(beanIdOrClassName);
71-
beanName = StringUtils.uncapitalize(beanClass.getSimpleName());
72-
} catch (ClassNotFoundException classNotFoundException) {
73-
beanName = beanIdOrClassName;
74-
}
75-
try {
76-
bean = beanFactory.getBean(beanName);
77-
} catch (BeansException beansException) {
78-
bean = beanFactory.getBean(beanClass);
79-
}
80-
byte b;
81-
int i;
82-
Method[] arrayOfMethod;
83-
for (i = (arrayOfMethod = bean.getClass().getMethods()).length, b = 0; b < i; ) {
84-
Method method = arrayOfMethod[b];
85-
if (!method.getName().equals(methodName)) {
86-
b++;
87-
continue;
88-
}
89-
ProxygenSerializer serializer = new ProxygenSerializer();
90-
Object[] methodInput = serializer.deserializeMethodInput(data, files, method);
91-
Object result = method.invoke(bean, methodInput);
92-
Map<String, Object> map = new HashMap<>();
93-
map.put("result", serializer.serialize(result));
94-
return map;
95-
}
96-
} catch (Exception e) {
97-
return e;
98-
}
99-
return null;
65+
try {
66+
Object bean = null;
67+
Class<?> beanClass = Class.forName(beanIdOrClassName);
68+
bean = beanFactory.getBean(beanClass);
69+
byte b;
70+
int i;
71+
Method[] arrayOfMethod;
72+
for (i = (arrayOfMethod = bean.getClass().getMethods()).length, b = 0; b < i; ) {
73+
Method method = arrayOfMethod[b];
74+
if (!method.getName().equals(methodName)) {
75+
b++;
76+
continue;
77+
}
78+
Object result = method.invoke(bean, data);
79+
Map<String, Object> map = new HashMap<>();
80+
return map;
81+
}
82+
} catch (Exception e) {
83+
return e;
84+
}
85+
return null;
10086
}
10187
}
10288

103-
class BeansException extends Exception {
104-
105-
}
106-
10789
class BeanFactory {
10890

109-
private static HashMap<String, Object> classNameMap = new HashMap<>();
110-
111-
private static HashMap<Class<?>, Object> classMap = new HashMap<>();;
112-
113-
static {
114-
classNameMap.put("xxxx", Runtime.getRuntime());
115-
classMap.put(Runtime.class, Runtime.getRuntime());
116-
}
91+
private static HashMap<String, Object> classNameMap = new HashMap<>();
11792

118-
public Object getBean(String className) throws BeansException {
119-
if (classNameMap.get(className) == null) {
120-
throw new BeansException();
121-
}
122-
return classNameMap.get(className);
123-
}
93+
private static HashMap<Class<?>, Object> classMap = new HashMap<>();
12494

125-
public Object getBean(Class<?> clzz) {
126-
return classMap.get(clzz);
127-
}
128-
}
95+
static {
96+
classNameMap.put("xxxx", Runtime.getRuntime());
97+
classMap.put(Runtime.class, Runtime.getRuntime());
98+
}
12999

130-
class ProxygenSerializer {
131-
132-
public Object[] deserializeMethodInput(List<Object> data, MultipartFile[] files, Method method) {
133-
return null;
134-
}
135-
136-
public String serialize(Object result) {
137-
return null;
138-
}
100+
public Object getBean(Class<?> clzz) {
101+
return classMap.get(clzz);
102+
}
139103
}

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

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -55,16 +55,9 @@ class UnsafeReflectionConfig extends TaintTracking::Configuration {
5555
ma = succ.asExpr()
5656
)
5757
or
58-
exists(MethodAccess ma, Method m, int i, Expr arg |
59-
m = ma.getMethod() and arg = ma.getArgument(i)
58+
exists(
59+
MethodAccess ma // Object.getClass()
6060
|
61-
m.getReturnType() instanceof TypeObject and
62-
arg.getType() instanceof TypeClass and
63-
arg = pred.asExpr() and
64-
ma = succ.asExpr()
65-
)
66-
or
67-
exists(MethodAccess ma |
6861
ma.getMethod().hasName("getClass") and
6962
ma.getMethod().getDeclaringType().hasQualifiedName("java.lang", "Object") and
7063
ma.getQualifier() = pred.asExpr() and
@@ -79,6 +72,15 @@ class UnsafeReflectionConfig extends TaintTracking::Configuration {
7972
arg = pred.asExpr() and
8073
ma = succ.asExpr()
8174
)
75+
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()
83+
)
8284
}
8385

8486
override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {

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

Lines changed: 8 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,8 @@
11
import java
22
import DataFlow
33
import semmle.code.java.Reflection
4-
import semmle.code.java.dataflow.DataFlow
54
import semmle.code.java.dataflow.DataFlow3
65
import semmle.code.java.dataflow.FlowSources
7-
import semmle.code.java.dataflow.TaintTracking
86
import semmle.code.java.dataflow.TaintTracking2
97

108
/**
@@ -44,18 +42,10 @@ class ReflectionArgsConfig extends TaintTracking2::Configuration {
4442
override predicate isSink(DataFlow::Node sink) {
4543
exists(NewInstance ni | ni.getAnArgument() = sink.asExpr())
4644
or
47-
exists(MethodAccess ma, ReflectionInvokeObjectConfig rioc |
45+
exists(MethodAccess ma |
4846
ma.getMethod().hasQualifiedName("java.lang.reflect", "Method", "invoke") and
4947
ma.getArgument(1) = sink.asExpr() and
50-
rioc.hasFlow(_, DataFlow::exprNode(ma.getArgument(0)))
51-
)
52-
}
53-
54-
override predicate isAdditionalTaintStep(Node pred, Node succ) {
55-
exists(MethodAccess ma |
56-
ma.getMethod().getReturnType().hasName("Object[]") and
57-
ma.getAnArgument() = pred.asExpr() and
58-
ma = succ.asExpr()
48+
exists(ReflectionInvokeObjectConfig rioc | rioc.hasFlowToExpr(ma.getArgument(0)))
5949
)
6050
}
6151
}
@@ -81,13 +71,12 @@ class ReflectionInvokeObjectConfig extends DataFlow3::Configuration {
8171
ni = succ.asExpr()
8272
)
8373
or
84-
exists(MethodAccess ma |
85-
ma.getMethod().getReturnType() instanceof TypeObject and
86-
(
87-
ma.getMethod().getAParamType() instanceof TypeString or
88-
ma.getMethod().getAParamType() instanceof TypeClass
89-
) and
90-
ma.getAnArgument() = pred.asExpr() and
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
9180
ma = succ.asExpr()
9281
)
9382
}

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,10 @@ edges
77
| UnsafeReflection.java:39:13:39:38 | getDeclaredMethods(...) : Method[] | UnsafeReflection.java:39:13:39:41 | ...[...] |
88
| UnsafeReflection.java:46:24:46:82 | beanIdOrClassName : String | UnsafeReflection.java:53:30:53:46 | beanIdOrClassName : String |
99
| 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:119:44:119:52 | beanClass : Class |
11-
| UnsafeReflection.java:104:34:104:57 | beanIdOrClassName : String | UnsafeReflection.java:132:33:132:38 | method |
12-
| UnsafeReflection.java:119:24:119:53 | getBean(...) : Object | UnsafeReflection.java:132:33:132:38 | method |
13-
| UnsafeReflection.java:119:44:119:52 | beanClass : Class | UnsafeReflection.java:119:24:119:53 | getBean(...) : Object |
10+
| UnsafeReflection.java:104:34:104:57 | beanIdOrClassName : String | UnsafeReflection.java:109:31:109:39 | beanClass : Class |
11+
| 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 |
1414
nodes
1515
| UnsafeReflection.java:21:28:21:60 | getParameter(...) : String | semmle.label | getParameter(...) : String |
1616
| UnsafeReflection.java:25:29:25:59 | getDeclaredConstructors(...) : Constructor[] | semmle.label | getDeclaredConstructors(...) : Constructor[] |
@@ -21,10 +21,10 @@ nodes
2121
| UnsafeReflection.java:46:24:46:82 | beanIdOrClassName : String | semmle.label | beanIdOrClassName : String |
2222
| UnsafeReflection.java:53:30:53:46 | beanIdOrClassName : String | semmle.label | beanIdOrClassName : String |
2323
| UnsafeReflection.java:104:34:104:57 | beanIdOrClassName : String | semmle.label | beanIdOrClassName : String |
24-
| UnsafeReflection.java:119:24:119:53 | getBean(...) : Object | semmle.label | getBean(...) : Object |
25-
| UnsafeReflection.java:119:44:119:52 | beanClass : Class | semmle.label | beanClass : Class |
26-
| UnsafeReflection.java:132:33:132:38 | method | semmle.label | method |
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 |
26+
| UnsafeReflection.java:119:21:119:26 | method | semmle.label | method |
2727
#select
2828
| 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 |
2929
| 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 |
30-
| UnsafeReflection.java:132:33:132:38 | method | UnsafeReflection.java:46:24:46:82 | beanIdOrClassName : String | UnsafeReflection.java:132:33:132:38 | method | Unsafe reflection of $@. | UnsafeReflection.java:46:24:46:82 | beanIdOrClassName | user input |
30+
| UnsafeReflection.java:119:21:119:26 | method | UnsafeReflection.java:46:24:46:82 | beanIdOrClassName : String | UnsafeReflection.java:119:21:119:26 | method | Unsafe reflection of $@. | UnsafeReflection.java:46:24:46:82 | beanIdOrClassName | user input |

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

Lines changed: 30 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -103,78 +103,42 @@ public void good3(HttpServletRequest request) throws Exception {
103103

104104
private Object invokeService(String beanIdOrClassName, String methodName, MultipartFile[] files, List<Object> data) throws Exception {
105105
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;
106+
try {
107+
Object bean = null;
108+
Class<?> beanClass = Class.forName(beanIdOrClassName);
109+
bean = beanFactory.getBean(beanClass);
110+
byte b;
111+
int i;
112+
Method[] arrayOfMethod;
113+
for (i = (arrayOfMethod = bean.getClass().getMethods()).length, b = 0; b < i; ) {
114+
Method method = arrayOfMethod[b];
115+
if (!method.getName().equals(methodName)) {
116+
b++;
117+
continue;
118+
}
119+
Object result = method.invoke(bean, data);
120+
Map<String, Object> map = new HashMap<>();
121+
return map;
122+
}
123+
} catch (Exception e) {
124+
return e;
125+
}
126+
return null;
141127
}
142128
}
143129

144-
class BeansException extends Exception {
145-
146-
}
147-
148130
class BeanFactory {
149131

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-
}
132+
private static HashMap<String, Object> classNameMap = new HashMap<>();
158133

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-
}
134+
private static HashMap<Class<?>, Object> classMap = new HashMap<>();
165135

166-
public Object getBean(Class<?> clzz) {
167-
return classMap.get(clzz);
168-
}
169-
}
136+
static {
137+
classNameMap.put("xxxx", Runtime.getRuntime());
138+
classMap.put(Runtime.class, Runtime.getRuntime());
139+
}
170140

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;
179-
}
141+
public Object getBean(Class<?> clzz) {
142+
return classMap.get(clzz);
143+
}
180144
}

0 commit comments

Comments
 (0)