mirror of
https://github.com/github/codeql.git
synced 2025-12-24 12:46:34 +01:00
Modify according to suggestions
This commit is contained in:
@@ -16,34 +16,8 @@ import org.springframework.web.multipart.MultipartFile;
|
||||
@Controller
|
||||
public class UnsafeReflection {
|
||||
|
||||
@GetMapping(value = "uf1")
|
||||
public void bad1(HttpServletRequest request) {
|
||||
String className = request.getParameter("className");
|
||||
String parameterValue = request.getParameter("parameterValue");
|
||||
try {
|
||||
Class clazz = Class.forName(className);
|
||||
Object object = clazz.getDeclaredConstructors()[0].newInstance(parameterValue); //bad
|
||||
} catch (Exception e) {
|
||||
e.printStackTrace();
|
||||
}
|
||||
}
|
||||
|
||||
@GetMapping(value = "uf2")
|
||||
public void bad2(HttpServletRequest request) {
|
||||
String className = request.getParameter("className");
|
||||
String parameterValue = request.getParameter("parameterValue");
|
||||
try {
|
||||
ClassLoader classLoader = ClassLoader.getSystemClassLoader();
|
||||
Class clazz = classLoader.loadClass(className);
|
||||
Object object = clazz.newInstance();
|
||||
clazz.getDeclaredMethods()[0].invoke(object, parameterValue); //bad
|
||||
} catch (Exception e) {
|
||||
e.printStackTrace();
|
||||
}
|
||||
}
|
||||
|
||||
@RequestMapping(value = {"/service/{beanIdOrClassName}/{methodName}"}, method = {RequestMethod.POST}, consumes = {"application/json"}, produces = {"application/json"})
|
||||
public Object bad3(@PathVariable("beanIdOrClassName") String beanIdOrClassName, @PathVariable("methodName") String methodName, @RequestBody Map<String, Object> body) throws Exception {
|
||||
public Object bad1(@PathVariable("beanIdOrClassName") String beanIdOrClassName, @PathVariable("methodName") String methodName, @RequestBody Map<String, Object> body) throws Exception {
|
||||
List<Object> rawData = null;
|
||||
try {
|
||||
rawData = (List<Object>)body.get("methodInput");
|
||||
@@ -53,7 +27,7 @@ public class UnsafeReflection {
|
||||
return invokeService(beanIdOrClassName, methodName, null, rawData);
|
||||
}
|
||||
|
||||
@GetMapping(value = "uf3")
|
||||
@GetMapping(value = "uf1")
|
||||
public void good1(HttpServletRequest request) throws Exception {
|
||||
HashSet<String> hashSet = new HashSet<>();
|
||||
hashSet.add("com.example.test1");
|
||||
@@ -71,7 +45,7 @@ public class UnsafeReflection {
|
||||
}
|
||||
}
|
||||
|
||||
@GetMapping(value = "uf4")
|
||||
@GetMapping(value = "uf2")
|
||||
public void good2(HttpServletRequest request) throws Exception {
|
||||
String className = request.getParameter("className");
|
||||
String parameterValue = request.getParameter("parameterValue");
|
||||
@@ -86,21 +60,6 @@ public class UnsafeReflection {
|
||||
}
|
||||
}
|
||||
|
||||
@GetMapping(value = "uf5")
|
||||
public void good3(HttpServletRequest request) throws Exception {
|
||||
String className = request.getParameter("className");
|
||||
String parameterValue = request.getParameter("parameterValue");
|
||||
if (!className.equals("com.example.test1")){ //good
|
||||
throw new Exception("Class not valid: " + className);
|
||||
}
|
||||
try {
|
||||
Class clazz = Class.forName(className);
|
||||
Object object = clazz.getDeclaredConstructors()[0].newInstance(parameterValue); //good
|
||||
} catch (Exception e) {
|
||||
e.printStackTrace();
|
||||
}
|
||||
}
|
||||
|
||||
private Object invokeService(String beanIdOrClassName, String methodName, MultipartFile[] files, List<Object> data) throws Exception {
|
||||
BeanFactory beanFactory = new BeanFactory();
|
||||
try {
|
||||
|
||||
@@ -3,26 +3,21 @@
|
||||
|
||||
<overview>
|
||||
<p>
|
||||
Allowing users to freely choose to call the constructor or method of the class through reflection. This means that
|
||||
malicious users can carefully construct requests to create instances of any class or call methods of any class.
|
||||
Allowing users to freely choose the name of a class to instantiate could provide means to attack a vulnerable appplication.
|
||||
</p>
|
||||
</overview>
|
||||
|
||||
<recommendation>
|
||||
<p>
|
||||
Create a list of classes that are allowed to load reflectively and strictly verify the input content to ensure that
|
||||
users can only execute classes or codes that are running.
|
||||
Create a list of classes that are allowed to load reflectively and strictly verify the input to ensure that
|
||||
users can only instantiate classes or execute methods that ought to be allowed.
|
||||
</p>
|
||||
</recommendation>
|
||||
|
||||
<example>
|
||||
<p>
|
||||
The following examples are good examples and bad examples. When using <code>Class.forName(...)</code> or
|
||||
<code>ClassLoader.loadClass(...)</code> to get the class, then create the class object or call the method of the class.
|
||||
When the user is not authenticated Input, it is easy to cause security risks, for example: <code>bad1</code>/<code>bad2
|
||||
</code>/<code>bad3</code>. When the user input is verified by <code>contains</code> or <code>equals</code> and then
|
||||
the reflection operation is performed, the system security can be well controlled, for example:
|
||||
<code >good1</code>/<code>good2</code>/<code>good3</code>.
|
||||
The <code>bad</code> method shown below illustrate class loading with <code>Class.forName</code> without any check on the particular class being instantiated.
|
||||
The <code>good</code> methods illustrate some different ways to restrict which classes can be instantiated.
|
||||
</p>
|
||||
<sample src="UnsafeReflection.java" />
|
||||
|
||||
|
||||
@@ -55,10 +55,12 @@ class UnsafeReflectionConfig extends TaintTracking::Configuration {
|
||||
ma = succ.asExpr()
|
||||
)
|
||||
or
|
||||
exists(MethodAccess ma |
|
||||
ma.getMethod().getReturnType() instanceof TypeObject and
|
||||
ma.getMethod().getAParamType() instanceof TypeClass and
|
||||
ma.getAnArgument() = pred.asExpr() and
|
||||
exists(MethodAccess ma, Method m, int i, Expr arg |
|
||||
m = ma.getMethod() and arg = ma.getArgument(i)
|
||||
|
|
||||
m.getReturnType() instanceof TypeObject and
|
||||
arg.getType() instanceof TypeClass and
|
||||
arg = pred.asExpr() and
|
||||
ma = succ.asExpr()
|
||||
)
|
||||
or
|
||||
@@ -72,8 +74,7 @@ class UnsafeReflectionConfig extends TaintTracking::Configuration {
|
||||
exists(MethodAccess ma, Method m, int i, Expr arg |
|
||||
m = ma.getMethod() and arg = ma.getArgument(i)
|
||||
|
|
||||
m.getReturnType() instanceof JacksonTypeDescriptorType and
|
||||
m.getName().toLowerCase().regexpMatch("resolve|load|class|type") and
|
||||
m.getReturnType() instanceof TypeClass and
|
||||
arg.getType() instanceof TypeString and
|
||||
arg = pred.asExpr() and
|
||||
ma = succ.asExpr()
|
||||
|
||||
@@ -7,15 +7,6 @@ import semmle.code.java.dataflow.FlowSources
|
||||
import semmle.code.java.dataflow.TaintTracking
|
||||
import semmle.code.java.dataflow.TaintTracking2
|
||||
|
||||
/** Type descriptors in Jackson libraries. */
|
||||
class JacksonTypeDescriptorType extends RefType {
|
||||
JacksonTypeDescriptorType() {
|
||||
this instanceof TypeClass or
|
||||
hasQualifiedName("com.fasterxml.jackson.databind", "JavaType") or
|
||||
hasQualifiedName("com.fasterxml.jackson.core.type", "TypeReference")
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* A call to a Java standard library method which constructs or returns a `Class<T>` from a `String`.
|
||||
* e.g `Class.forName(...)` or `ClassLoader.loadClass(...)`
|
||||
|
||||
Reference in New Issue
Block a user