hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-26 09:15:12 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #5818 from artem-smotrakov/rmi-deserialization

Browse Source 
Java: Unsafe RMI deserialization
This commit is contained in:
Chris Smowton
2021-06-11 13:43:07 +01:00
committed by GitHub
parent 97486b448a 8dc1451d42
commit 76838809bb
8 changed files with 284 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
java/ql/src/experimental/Security/CWE/CWE-502/RmiRemoteObjectWithFilter.java Normal file
View File

@@ -0,0 +1,9 @@
public void bindRemoteObject(Registry registry, int port) throws Exception {
ObjectInputFilter filter = info -> {
if (info.serialClass().getCanonicalName().startsWith("com.safe.package.")) {
return ObjectInputFilter.Status.ALLOWED;
}
return ObjectInputFilter.Status.REJECTED;
};
registry.bind("safer", UnicastRemoteObject.exportObject(new RemoteObjectImpl(), port, filter));
}

14
java/ql/src/experimental/Security/CWE/CWE-502/RmiSafeRemoteObject.java Normal file
View File

@@ -0,0 +1,14 @@
public class Server {
public void bindRemoteObject(Registry registry) throws Exception {
registry.bind("safe", new RemoteObjectImpl());
}
}
interface RemoteObject extends Remote {
void calculate(int a, double b) throws RemoteException;
void save(String s) throws RemoteException;
}
class RemoteObjectImpl implements RemoteObject {
// ...
}

13
java/ql/src/experimental/Security/CWE/CWE-502/RmiUnsafeRemoteObject.java Normal file
View File

@@ -0,0 +1,13 @@
public class Server {
public void bindRemoteObject(Registry registry) throws Exception {
registry.bind("unsafe", new RemoteObjectImpl());
}
}
interface RemoteObject extends Remote {
void action(Object obj) throws RemoteException;
}
class RemoteObjectImpl implements RemoteObject {
// ...
}

81
java/ql/src/experimental/Security/CWE/CWE-502/UnsafeDeserializationRmi.qhelp Normal file
View File

@@ -0,0 +1,81 @@
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
<qhelp>
<overview>
<p>
Java RMI uses the default Java serialization mechanism (in other words, <code>ObjectInputStream</code>)
to pass parameters in remote method invocations. This mechanism is known to be unsafe when deserializing
untrusted data. If a registered remote object has a method that accepts a complex object,
an attacker can take advantage of the unsafe deserialization mechanism.
In the worst case, it results in remote code execution.
</p>
</overview>
<recommendation>
<p>
Use only strings and primitive types for parameters of remotely invokable methods.
</p>
<p>
Set a filter for incoming serialized data by wrapping remote objects using either <code>UnicastRemoteObject.exportObject(Remote, int, ObjectInputFilter)</code>
or <code>UnicastRemoteObject.exportObject(Remote, int, RMIClientSocketFactory, RMIServerSocketFactory, ObjectInputFilter)</code> methods.
Those methods accept an <code>ObjectInputFilter</code> that decides which classes are allowed for deserialization.
The filter should allow deserializing only safe classes.
</p>
<p>
It is also possible to set a process-wide deserialization filter.
The filter can be set by with <code>ObjectInputFilter.Config.setSerialFilter(ObjectInputFilter)</code> method,
or by setting system or security property <code>jdk.serialFilter</code>.
Make sure that you use the latest Java versions that include JEP 290.
Please note that the query is not sensitive to this mitigation.
</p>
<p>
If switching to the latest Java versions is not possible,
consider using other implementations of remote procedure calls. For example, HTTP API with JSON.
Make sure that the underlying deserialization mechanism is properly configured
so that deserialization attacks are not possible.
</p>
</recommendation>
<example>
<p>
The following code registers a remote object
with a vulnerable method that accepts a complex object:
</p>
<sample src="RmiUnsafeRemoteObject.java" />
<p>
The next example registers a safe remote object
whose methods use only primitive types and strings:
</p>
<sample src="RmiSafeRemoteObject.java" />
<p>
The next example shows how to set a deserilization filter for a remote object:
</p>
<sample src="RmiRemoteObjectWithFilter.java" />
</example>
<references>
<li>
Oracle:
<a href="https://www.oracle.com/java/technologies/javase/remote-method-invocation-home.html">Remote Method Invocation (RMI)</a>.
</li>
<li>
ITNEXT:
<a href="https://itnext.io/java-rmi-for-pentesters-part-two-reconnaissance-attack-against-non-jmx-registries-187a6561314d">Java RMI for pentesters part two - reconnaissance &amp; attack against non-JMX registries</a>.
</li>
<li>
MOGWAI LABS:
<a href="https://mogwailabs.de/en/blog/2019/03/attacking-java-rmi-services-after-jep-290">Attacking Java RMI services after JEP 290</a>
</li>
<li>
OWASP:
<a href="https://www.owasp.org/index.php/Deserialization_of_untrusted_data">Deserialization of untrusted data</a>.
</li>
<li>
OpenJDK:
<a href="https://openjdk.java.net/jeps/290">JEP 290: Filter Incoming Serialization Data</a>
</li>
</references>
</qhelp>

78
java/ql/src/experimental/Security/CWE/CWE-502/UnsafeDeserializationRmi.ql Normal file
View File

@@ -0,0 +1,78 @@
/**
* @name Unsafe deserialization in a remotely callable method.
* @description If a registered remote object has a method that accepts a complex object,
* an attacker can take advantage of the unsafe deserialization mechanism
* which is used to pass parameters in RMI.
* In the worst case, it results in remote code execution.
* @kind path-problem
* @problem.severity error
* @precision high
* @id java/unsafe-deserialization-rmi
* @tags security
* external/cwe/cwe-502
*/
import java
import semmle.code.java.dataflow.TaintTracking
import semmle.code.java.frameworks.Rmi
import DataFlow::PathGraph
/**
* A method that binds a name to a remote object.
*/
private class BindMethod extends Method {
BindMethod() {
(
getDeclaringType().hasQualifiedName("java.rmi", "Naming") or
getDeclaringType().hasQualifiedName("java.rmi.registry", "Registry")
) and
hasName(["bind", "rebind"])
}
}
/**
* Holds if `type` has an vulnerable remote method.
*/
private predicate hasVulnerableMethod(RefType type) {
exists(RemoteCallableMethod m, Type parameterType |
m.getDeclaringType() = type and parameterType = m.getAParamType()
|
not parameterType instanceof PrimitiveType and
not parameterType instanceof TypeString and
not parameterType.(RefType).hasQualifiedName("java.io", "ObjectInputStream")
)
}
/**
* A taint-tracking configuration for unsafe remote objects
* that are vulnerable to deserialization attacks.
*/
private class BindingUnsafeRemoteObjectConfig extends TaintTracking::Configuration {
BindingUnsafeRemoteObjectConfig() { this = "BindingUnsafeRemoteObjectConfig" }
override predicate isSource(DataFlow::Node source) {
exists(ConstructorCall cc | cc = source.asExpr() |
hasVulnerableMethod(cc.getConstructedType().getASupertype*())
)
}
override predicate isSink(DataFlow::Node sink) {
exists(MethodAccess ma | ma.getArgument(1) = sink.asExpr() |
ma.getMethod() instanceof BindMethod
)
}
override predicate isAdditionalTaintStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
exists(MethodAccess ma, Method m | m = ma.getMethod() |
m.getDeclaringType().hasQualifiedName("java.rmi.server", "UnicastRemoteObject") and
m.hasName("exportObject") and
not m.getParameterType([2, 4]).(RefType).hasQualifiedName("java.io", "ObjectInputFilter") and
ma.getArgument(0) = fromNode.asExpr() and
ma = toNode.asExpr()
)
}
}
from DataFlow::PathNode source, DataFlow::PathNode sink, BindingUnsafeRemoteObjectConfig conf
where conf.hasFlowPath(source, sink)
select sink.getNode(), source, sink, "Unsafe deserialization in a remote object."

15
java/ql/test/experimental/query-tests/security/CWE-502/UnsafeDeserializationRmi.expected Normal file
View File

@@ -0,0 +1,15 @@
edges
| UnsafeDeserializationRmi.java:17:68:17:95 | new UnsafeRemoteObjectImpl(...) : UnsafeRemoteObjectImpl | UnsafeDeserializationRmi.java:17:35:17:96 | exportObject(...) |
nodes
| UnsafeDeserializationRmi.java:15:33:15:60 | new UnsafeRemoteObjectImpl(...) | semmle.label | new UnsafeRemoteObjectImpl(...) |
| UnsafeDeserializationRmi.java:16:35:16:62 | new UnsafeRemoteObjectImpl(...) | semmle.label | new UnsafeRemoteObjectImpl(...) |
| UnsafeDeserializationRmi.java:17:35:17:96 | exportObject(...) | semmle.label | exportObject(...) |
| UnsafeDeserializationRmi.java:17:68:17:95 | new UnsafeRemoteObjectImpl(...) : UnsafeRemoteObjectImpl | semmle.label | new UnsafeRemoteObjectImpl(...) : UnsafeRemoteObjectImpl |
| UnsafeDeserializationRmi.java:29:31:29:58 | new UnsafeRemoteObjectImpl(...) | semmle.label | new UnsafeRemoteObjectImpl(...) |
| UnsafeDeserializationRmi.java:30:33:30:60 | new UnsafeRemoteObjectImpl(...) | semmle.label | new UnsafeRemoteObjectImpl(...) |
#select
| UnsafeDeserializationRmi.java:15:33:15:60 | new UnsafeRemoteObjectImpl(...) | UnsafeDeserializationRmi.java:15:33:15:60 | new UnsafeRemoteObjectImpl(...) | UnsafeDeserializationRmi.java:15:33:15:60 | new UnsafeRemoteObjectImpl(...) | Unsafe deserialization in a remote object. |
| UnsafeDeserializationRmi.java:16:35:16:62 | new UnsafeRemoteObjectImpl(...) | UnsafeDeserializationRmi.java:16:35:16:62 | new UnsafeRemoteObjectImpl(...) | UnsafeDeserializationRmi.java:16:35:16:62 | new UnsafeRemoteObjectImpl(...) | Unsafe deserialization in a remote object. |
| UnsafeDeserializationRmi.java:17:35:17:96 | exportObject(...) | UnsafeDeserializationRmi.java:17:68:17:95 | new UnsafeRemoteObjectImpl(...) : UnsafeRemoteObjectImpl | UnsafeDeserializationRmi.java:17:35:17:96 | exportObject(...) | Unsafe deserialization in a remote object. |
| UnsafeDeserializationRmi.java:29:31:29:58 | new UnsafeRemoteObjectImpl(...) | UnsafeDeserializationRmi.java:29:31:29:58 | new UnsafeRemoteObjectImpl(...) | UnsafeDeserializationRmi.java:29:31:29:58 | new UnsafeRemoteObjectImpl(...) | Unsafe deserialization in a remote object. |
| UnsafeDeserializationRmi.java:30:33:30:60 | new UnsafeRemoteObjectImpl(...) | UnsafeDeserializationRmi.java:30:33:30:60 | new UnsafeRemoteObjectImpl(...) | UnsafeDeserializationRmi.java:30:33:30:60 | new UnsafeRemoteObjectImpl(...) | Unsafe deserialization in a remote object. |

73
java/ql/test/experimental/query-tests/security/CWE-502/UnsafeDeserializationRmi.java Normal file
View File

@@ -0,0 +1,73 @@
import java.io.ObjectInputFilter;
import java.io.ObjectInputStream;
import java.rmi.Naming;
import java.rmi.Remote;
import java.rmi.RemoteException;
import java.rmi.registry.LocateRegistry;
import java.rmi.registry.Registry;
import java.rmi.server.UnicastRemoteObject;
public class UnsafeDeserializationRmi {
// BAD (bind a remote object that has a vulnerable method)
public static void testRegistryBindWithObjectParameter() throws Exception {
Registry registry = LocateRegistry.createRegistry(1099);
registry.bind("unsafe", new UnsafeRemoteObjectImpl());
registry.rebind("unsafe", new UnsafeRemoteObjectImpl());
registry.rebind("unsafe", UnicastRemoteObject.exportObject(new UnsafeRemoteObjectImpl()));
}
// GOOD (bind a remote object that has methods that takes safe parameters)
public static void testRegistryBindWithIntParameter() throws Exception {
Registry registry = LocateRegistry.createRegistry(1099);
registry.bind("safe", new SafeRemoteObjectImpl());
registry.rebind("safe", new SafeRemoteObjectImpl());
}
// BAD (bind a remote object that has a vulnerable method)
public static void testNamingBindWithObjectParameter() throws Exception {
Naming.bind("unsafe", new UnsafeRemoteObjectImpl());
Naming.rebind("unsafe", new UnsafeRemoteObjectImpl());
}
// GOOD (bind a remote object that has methods that takes safe parameters)
public static void testNamingBindWithIntParameter() throws Exception {
Naming.bind("safe", new SafeRemoteObjectImpl());
Naming.rebind("safe", new SafeRemoteObjectImpl());
}
// GOOD (bind a remote object with a deserialization filter)
public static void testRegistryBindWithDeserializationFilter() throws Exception {
Registry registry = LocateRegistry.createRegistry(1099);
ObjectInputFilter filter = info -> {
if (info.serialClass().getCanonicalName().startsWith("com.safe.package.")) {
return ObjectInputFilter.Status.ALLOWED;
}
return ObjectInputFilter.Status.REJECTED;
};
registry.rebind("safe", UnicastRemoteObject.exportObject(new UnsafeRemoteObjectImpl(), 12345, filter));
}
}
interface UnsafeRemoteObject extends Remote {
void take(Object obj) throws RemoteException;
}
class UnsafeRemoteObjectImpl implements UnsafeRemoteObject {
public void take(Object obj) throws RemoteException {}
}
interface SafeRemoteObject extends Remote {
void take(int n) throws RemoteException;
void take(double n) throws RemoteException;
void take(String s) throws RemoteException;
void take(ObjectInputStream ois) throws RemoteException;
}
class SafeRemoteObjectImpl implements SafeRemoteObject {
public void take(int n) throws RemoteException {}
public void take(double n) throws RemoteException {}
public void take(String s) throws RemoteException {}
public void take(ObjectInputStream ois) throws RemoteException {}
public void safeMethod(Object object) {} // this method is not declared in SafeRemoteObject
}

1
java/ql/test/experimental/query-tests/security/CWE-502/UnsafeDeserializationRmi.qlref Normal file
View File

@@ -0,0 +1 @@
experimental/Security/CWE/CWE-502/UnsafeDeserializationRmi.ql