Skip to content

Commit 4ebf0ed

Browse files
haby0haby0
committed
Use of Externally-Controlled Input to Select Classes or Code ('Unsafe Reflection')
1 parent 802d9bd commit 4ebf0ed

File tree

22 files changed

+583
-0
lines changed

22 files changed

+583
-0
lines changed

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

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
import java.util.HashSet;
2+
import javax.servlet.http.HttpServletRequest;
3+
import org.springframework.stereotype.Controller;
4+
import org.springframework.web.bind.annotation.GetMapping;
5+
import org.springframework.web.bind.annotation.ResponseBody;
6+
7+
@Controller
8+
public class UnsafeReflection {
9+
10+
@GetMapping(value = "uf1")
11+
public void bad1(HttpServletRequest request) {
12+
String className = request.getParameter("className");
13+
try {
14+
Class clazz = Class.forName(className); //bad
15+
} catch (ClassNotFoundException e) {
16+
e.printStackTrace();
17+
}
18+
}
19+
20+
@GetMapping(value = "uf2")
21+
public void bad2(HttpServletRequest request) {
22+
String className = request.getParameter("className");
23+
try {
24+
ClassLoader classLoader = ClassLoader.getSystemClassLoader();
25+
Class clazz = classLoader.loadClass(className); //bad
26+
} catch (ClassNotFoundException e) {
27+
e.printStackTrace();
28+
}
29+
}
30+
31+
@GetMapping(value = "uf3")
32+
public void good1(HttpServletRequest request) throws Exception {
33+
HashSet<String> hashSet = new HashSet<>();
34+
hashSet.add("com.example.test1");
35+
hashSet.add("com.example.test2");
36+
String className = request.getParameter("className");
37+
if (hashSet.contains(className)){ //good
38+
throw new Exception("Class not valid: " + className);
39+
}
40+
ClassLoader classLoader = ClassLoader.getSystemClassLoader();
41+
Class clazz = classLoader.loadClass(className);
42+
}
43+
44+
@GetMapping(value = "uf4")
45+
public void good2(HttpServletRequest request) throws Exception {
46+
String className = request.getParameter("className");
47+
if (!"com.example.test1".equals(className)){ //good
48+
throw new Exception("Class not valid: " + className);
49+
}
50+
ClassLoader classLoader = ClassLoader.getSystemClassLoader();
51+
Class clazz = classLoader.loadClass(className);
52+
}
53+
54+
@GetMapping(value = "uf5")
55+
public void good3(HttpServletRequest request) throws Exception {
56+
String className = request.getParameter("className");
57+
if (!className.equals("com.example.test1")){ //good
58+
throw new Exception("Class not valid: " + className);
59+
}
60+
ClassLoader classLoader = ClassLoader.getSystemClassLoader();
61+
Class clazz = classLoader.loadClass(className);
62+
}
63+
}

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

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
2+
<qhelp>
3+
4+
<overview>
5+
<p>
6+
Dynamically loaded classes could contain malicious code executed by a static class initializer.
7+
I.E. you wouldn't even have to instantiate or explicitly invoke methods on such classes to be
8+
vulnerable to an attack.
9+
</p>
10+
</overview>
11+
12+
<recommendation>
13+
<p>
14+
Create a whitelist of allowed reflection classes and strictly verify the input content to ensure that
15+
users can only execute classes or codes that are running.
16+
</p>
17+
</recommendation>
18+
19+
<example>
20+
<p>
21+
The following examples show good examples and bad examples respectively. When using <code>Class.forName(...)</code>
22+
or <code>ClassLoader.loadClass(...)</code> and not verifying user input, it is easy to cause security risks, for example:
23+
<code>bad1</code>/<code>bad2</code>. When the user input is verified by <code>contains</code> or <code>equals</code> and then
24+
the reflection operation is performed, the system security can be well controlled, for example:
25+
<code >good1</code>/<code>good2</code>/<code>good3</code>.
26+
</p>
27+
<sample src="UnsafeReflection.java" />
28+
29+
</example>
30+
31+
<references>
32+
33+
<li>
34+
Unsafe use of Reflection | OWASP:
35+
<a href="https://owasp.org/www-community/vulnerabilities/Unsafe_use_of_Reflection">Unsafe use of Reflection</a>.
36+
</li>
37+
<li>
38+
Java owasp: Classes should not be loaded dynamically:
39+
<a href="https://rules.sonarsource.com/java/tag/owasp/RSPEC-2658">Classes should not be loaded dynamically</a>.
40+
</li>
41+
</references>
42+
43+
</qhelp>

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

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
/**
2+
* @name Use of Externally-Controlled Input to Select Classes or Code ('Unsafe Reflection')
3+
* @description Use external input with reflection function to select the class or code to
4+
* be used, which brings serious security risks.
5+
* @kind path-problem
6+
* @problem.severity error
7+
* @precision high
8+
* @id java/unsafe-reflection
9+
* @tags security
10+
* external/cwe/cwe-470
11+
*/
12+
13+
import java
14+
import UnsafeReflectionLib
15+
import semmle.code.java.dataflow.FlowSources
16+
import DataFlow::PathGraph
17+
18+
class ContainsSanitizer extends DataFlow::BarrierGuard {
19+
ContainsSanitizer() { this.(MethodAccess).getMethod().hasName("contains") }
20+
21+
override predicate checks(Expr e, boolean branch) {
22+
e = this.(MethodAccess).getArgument(0) and branch = false
23+
}
24+
}
25+
26+
class EqualsSanitizer extends DataFlow::BarrierGuard {
27+
EqualsSanitizer() { this.(MethodAccess).getMethod().hasName("equals") }
28+
29+
override predicate checks(Expr e, boolean branch) {
30+
(e = this.(MethodAccess).getArgument(0) or e = this.(MethodAccess).getQualifier()) and
31+
branch = true
32+
}
33+
}
34+
35+
class UnsafeReflectionConfig extends TaintTracking::Configuration {
36+
UnsafeReflectionConfig() { this = "UnsafeReflectionConfig" }
37+
38+
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
39+
40+
override predicate isSink(DataFlow::Node sink) { sink instanceof UnsafeReflectionSink }
41+
42+
override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
43+
guard instanceof ContainsSanitizer or guard instanceof EqualsSanitizer
44+
}
45+
}
46+
47+
from DataFlow::PathNode source, DataFlow::PathNode sink, UnsafeReflectionConfig conf
48+
where conf.hasFlowPath(source, sink)
49+
select sink.getNode(), source, sink, "Unsafe reflection of $@.", source.getNode(), "user input"

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import java
2+
import semmle.code.java.Reflection
3+
import semmle.code.java.dataflow.DataFlow
4+
5+
/**
6+
* Unsafe reflection sink,
7+
* e.g `Class.forName(...)` or `ClassLoader.loadClass(...)`.
8+
*/
9+
class UnsafeReflectionSink extends DataFlow::ExprNode {
10+
UnsafeReflectionSink() {
11+
exists(ReflectiveClassIdentifierMethodAccess rcima | rcima.getArgument(0) = this.getExpr())
12+
}
13+
}

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
edges
2+
| UnsafeReflection.java:12:28:12:60 | getParameter(...) : String | UnsafeReflection.java:14:41:14:49 | className |
3+
| UnsafeReflection.java:22:28:22:60 | getParameter(...) : String | UnsafeReflection.java:25:49:25:57 | className |
4+
nodes
5+
| UnsafeReflection.java:12:28:12:60 | getParameter(...) : String | semmle.label | getParameter(...) : String |
6+
| UnsafeReflection.java:14:41:14:49 | className | semmle.label | className |
7+
| UnsafeReflection.java:22:28:22:60 | getParameter(...) : String | semmle.label | getParameter(...) : String |
8+
| UnsafeReflection.java:25:49:25:57 | className | semmle.label | className |
9+
#select
10+
| UnsafeReflection.java:14:41:14:49 | className | UnsafeReflection.java:12:28:12:60 | getParameter(...) : String | UnsafeReflection.java:14:41:14:49 | className | Unsafe reflection of $@. | UnsafeReflection.java:12:28:12:60 | getParameter(...) | user input |
11+
| UnsafeReflection.java:25:49:25:57 | className | UnsafeReflection.java:22:28:22:60 | getParameter(...) : String | UnsafeReflection.java:25:49:25:57 | className | Unsafe reflection of $@. | UnsafeReflection.java:22:28:22:60 | getParameter(...) | user input |

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

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
import java.util.HashSet;
2+
import javax.servlet.http.HttpServletRequest;
3+
import org.springframework.stereotype.Controller;
4+
import org.springframework.web.bind.annotation.GetMapping;
5+
import org.springframework.web.bind.annotation.ResponseBody;
6+
7+
@Controller
8+
public class UnsafeReflection {
9+
10+
@GetMapping(value = "uf1")
11+
public void bad1(HttpServletRequest request) {
12+
String className = request.getParameter("className");
13+
try {
14+
Class clazz = Class.forName(className); //bad
15+
} catch (ClassNotFoundException e) {
16+
e.printStackTrace();
17+
}
18+
}
19+
20+
@GetMapping(value = "uf2")
21+
public void bad2(HttpServletRequest request) {
22+
String className = request.getParameter("className");
23+
try {
24+
ClassLoader classLoader = ClassLoader.getSystemClassLoader();
25+
Class clazz = classLoader.loadClass(className); //bad
26+
} catch (ClassNotFoundException e) {
27+
e.printStackTrace();
28+
}
29+
}
30+
31+
@GetMapping(value = "uf3")
32+
public void good1(HttpServletRequest request) throws Exception {
33+
HashSet<String> hashSet = new HashSet<>();
34+
hashSet.add("com.example.test1");
35+
hashSet.add("com.example.test2");
36+
String className = request.getParameter("className");
37+
if (hashSet.contains(className)){ //good
38+
throw new Exception("Class not valid: " + className);
39+
}
40+
ClassLoader classLoader = ClassLoader.getSystemClassLoader();
41+
Class clazz = classLoader.loadClass(className);
42+
}
43+
44+
@GetMapping(value = "uf4")
45+
public void good2(HttpServletRequest request) throws Exception {
46+
String className = request.getParameter("className");
47+
if (!"com.example.test1".equals(className)){ //good
48+
throw new Exception("Class not valid: " + className);
49+
}
50+
ClassLoader classLoader = ClassLoader.getSystemClassLoader();
51+
Class clazz = classLoader.loadClass(className);
52+
}
53+
54+
@GetMapping(value = "uf5")
55+
public void good3(HttpServletRequest request) throws Exception {
56+
String className = request.getParameter("className");
57+
if (!className.equals("com.example.test1")){ //good
58+
throw new Exception("Class not valid: " + className);
59+
}
60+
ClassLoader classLoader = ClassLoader.getSystemClassLoader();
61+
Class clazz = classLoader.loadClass(className);
62+
}
63+
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
experimental/Security/CWE/CWE-470/UnsafeReflection.ql

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/servlet-api-2.4:${testdir}/../../../../stubs/spring-context-5.3.2/:${testdir}/../../../../stubs/spring-web-5.3.2/:${testdir}/../../../../stubs/spring-core-5.3.2/

‎java/ql/test/stubs/spring-context-5.3.2/org/springframework/stereotype/Controller.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
package org.springframework.stereotype;
2+
3+
import java.lang.annotation.Documented;
4+
import java.lang.annotation.ElementType;
5+
import java.lang.annotation.Retention;
6+
import java.lang.annotation.RetentionPolicy;
7+
import java.lang.annotation.Target;
8+
9+
@Target({ElementType.TYPE})
10+
@Retention(RetentionPolicy.RUNTIME)
11+
@Documented
12+
public @interface Controller {
13+
String value() default "";
14+
}

‎java/ql/test/stubs/spring-core-5.3.2/org/springframework/core/annotation/AliasFor.java

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
package org.springframework.core.annotation;
2+
3+
import java.lang.annotation.Annotation;
4+
import java.lang.annotation.Documented;
5+
import java.lang.annotation.ElementType;
6+
import java.lang.annotation.Retention;
7+
import java.lang.annotation.RetentionPolicy;
8+
import java.lang.annotation.Target;
9+
10+
@Retention(RetentionPolicy.RUNTIME)
11+
@Target({ElementType.METHOD})
12+
@Documented
13+
public @interface AliasFor {
14+
@AliasFor("attribute")
15+
String value() default "";
16+
17+
@AliasFor("value")
18+
String attribute() default "";
19+
20+
Class<? extends Annotation> annotation() default Annotation.class;
21+
}

0 commit comments

Comments
 (0)