Skip to content

Commit 7683880

Browse files
smowtonsmowton
authored
Merge pull request github#5818 from artem-smotrakov/rmi-deserialization
Java: Unsafe RMI deserialization
2 parents 97486b4 + 8dc1451 commit 7683880

File tree

8 files changed

+284
-0
lines changed

8 files changed

+284
-0
lines changed

‎java/ql/src/experimental/Security/CWE/CWE-502/RmiRemoteObjectWithFilter.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
public void bindRemoteObject(Registry registry, int port) throws Exception {
2+
ObjectInputFilter filter = info -> {
3+
if (info.serialClass().getCanonicalName().startsWith("com.safe.package.")) {
4+
return ObjectInputFilter.Status.ALLOWED;
5+
}
6+
return ObjectInputFilter.Status.REJECTED;
7+
};
8+
registry.bind("safer", UnicastRemoteObject.exportObject(new RemoteObjectImpl(), port, filter));
9+
}

‎java/ql/src/experimental/Security/CWE/CWE-502/RmiSafeRemoteObject.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
public class Server {
2+
public void bindRemoteObject(Registry registry) throws Exception {
3+
registry.bind("safe", new RemoteObjectImpl());
4+
}
5+
}
6+
7+
interface RemoteObject extends Remote {
8+
void calculate(int a, double b) throws RemoteException;
9+
void save(String s) throws RemoteException;
10+
}
11+
12+
class RemoteObjectImpl implements RemoteObject {
13+
// ...
14+
}

‎java/ql/src/experimental/Security/CWE/CWE-502/RmiUnsafeRemoteObject.java

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
public class Server {
2+
public void bindRemoteObject(Registry registry) throws Exception {
3+
registry.bind("unsafe", new RemoteObjectImpl());
4+
}
5+
}
6+
7+
interface RemoteObject extends Remote {
8+
void action(Object obj) throws RemoteException;
9+
}
10+
11+
class RemoteObjectImpl implements RemoteObject {
12+
// ...
13+
}

‎java/ql/src/experimental/Security/CWE/CWE-502/UnsafeDeserializationRmi.qhelp

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
2+
<qhelp>
3+
4+
<overview>
5+
<p>
6+
Java RMI uses the default Java serialization mechanism (in other words, <code>ObjectInputStream</code>)
7+
to pass parameters in remote method invocations. This mechanism is known to be unsafe when deserializing
8+
untrusted data. If a registered remote object has a method that accepts a complex object,
9+
an attacker can take advantage of the unsafe deserialization mechanism.
10+
In the worst case, it results in remote code execution.
11+
</p>
12+
</overview>
13+
14+
<recommendation>
15+
<p>
16+
Use only strings and primitive types for parameters of remotely invokable methods.
17+
</p>
18+
<p>
19+
Set a filter for incoming serialized data by wrapping remote objects using either <code>UnicastRemoteObject.exportObject(Remote, int, ObjectInputFilter)</code>
20+
or <code>UnicastRemoteObject.exportObject(Remote, int, RMIClientSocketFactory, RMIServerSocketFactory, ObjectInputFilter)</code> methods.
21+
Those methods accept an <code>ObjectInputFilter</code> that decides which classes are allowed for deserialization.
22+
The filter should allow deserializing only safe classes.
23+
</p>
24+
<p>
25+
It is also possible to set a process-wide deserialization filter.
26+
The filter can be set by with <code>ObjectInputFilter.Config.setSerialFilter(ObjectInputFilter)</code> method,
27+
or by setting system or security property <code>jdk.serialFilter</code>.
28+
Make sure that you use the latest Java versions that include JEP 290.
29+
Please note that the query is not sensitive to this mitigation.
30+
</p>
31+
<p>
32+
If switching to the latest Java versions is not possible,
33+
consider using other implementations of remote procedure calls. For example, HTTP API with JSON.
34+
Make sure that the underlying deserialization mechanism is properly configured
35+
so that deserialization attacks are not possible.
36+
</p>
37+
</recommendation>
38+
39+
<example>
40+
<p>
41+
The following code registers a remote object
42+
with a vulnerable method that accepts a complex object:
43+
</p>
44+
<sample src="RmiUnsafeRemoteObject.java" />
45+
46+
<p>
47+
The next example registers a safe remote object
48+
whose methods use only primitive types and strings:
49+
</p>
50+
<sample src="RmiSafeRemoteObject.java" />
51+
52+
<p>
53+
The next example shows how to set a deserilization filter for a remote object:
54+
</p>
55+
<sample src="RmiRemoteObjectWithFilter.java" />
56+
57+
</example>
58+
59+
<references>
60+
<li>
61+
Oracle:
62+
<a href="https://www.oracle.com/java/technologies/javase/remote-method-invocation-home.html">Remote Method Invocation (RMI)</a>.
63+
</li>
64+
<li>
65+
ITNEXT:
66+
<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>.
67+
</li>
68+
<li>
69+
MOGWAI LABS:
70+
<a href="https://mogwailabs.de/en/blog/2019/03/attacking-java-rmi-services-after-jep-290">Attacking Java RMI services after JEP 290</a>
71+
</li>
72+
<li>
73+
OWASP:
74+
<a href="https://www.owasp.org/index.php/Deserialization_of_untrusted_data">Deserialization of untrusted data</a>.
75+
</li>
76+
<li>
77+
OpenJDK:
78+
<a href="https://openjdk.java.net/jeps/290">JEP 290: Filter Incoming Serialization Data</a>
79+
</li>
80+
</references>
81+
</qhelp>

‎java/ql/src/experimental/Security/CWE/CWE-502/UnsafeDeserializationRmi.ql

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
/**
2+
* @name Unsafe deserialization in a remotely callable method.
3+
* @description If a registered remote object has a method that accepts a complex object,
4+
* an attacker can take advantage of the unsafe deserialization mechanism
5+
* which is used to pass parameters in RMI.
6+
* In the worst case, it results in remote code execution.
7+
* @kind path-problem
8+
* @problem.severity error
9+
* @precision high
10+
* @id java/unsafe-deserialization-rmi
11+
* @tags security
12+
* external/cwe/cwe-502
13+
*/
14+
15+
import java
16+
import semmle.code.java.dataflow.TaintTracking
17+
import semmle.code.java.frameworks.Rmi
18+
import DataFlow::PathGraph
19+
20+
/**
21+
* A method that binds a name to a remote object.
22+
*/
23+
private class BindMethod extends Method {
24+
BindMethod() {
25+
(
26+
getDeclaringType().hasQualifiedName("java.rmi", "Naming") or
27+
getDeclaringType().hasQualifiedName("java.rmi.registry", "Registry")
28+
) and
29+
hasName(["bind", "rebind"])
30+
}
31+
}
32+
33+
/**
34+
* Holds if `type` has an vulnerable remote method.
35+
*/
36+
private predicate hasVulnerableMethod(RefType type) {
37+
exists(RemoteCallableMethod m, Type parameterType |
38+
m.getDeclaringType() = type and parameterType = m.getAParamType()
39+
|
40+
not parameterType instanceof PrimitiveType and
41+
not parameterType instanceof TypeString and
42+
not parameterType.(RefType).hasQualifiedName("java.io", "ObjectInputStream")
43+
)
44+
}
45+
46+
/**
47+
* A taint-tracking configuration for unsafe remote objects
48+
* that are vulnerable to deserialization attacks.
49+
*/
50+
private class BindingUnsafeRemoteObjectConfig extends TaintTracking::Configuration {
51+
BindingUnsafeRemoteObjectConfig() { this = "BindingUnsafeRemoteObjectConfig" }
52+
53+
override predicate isSource(DataFlow::Node source) {
54+
exists(ConstructorCall cc | cc = source.asExpr() |
55+
hasVulnerableMethod(cc.getConstructedType().getASupertype*())
56+
)
57+
}
58+
59+
override predicate isSink(DataFlow::Node sink) {
60+
exists(MethodAccess ma | ma.getArgument(1) = sink.asExpr() |
61+
ma.getMethod() instanceof BindMethod
62+
)
63+
}
64+
65+
override predicate isAdditionalTaintStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
66+
exists(MethodAccess ma, Method m | m = ma.getMethod() |
67+
m.getDeclaringType().hasQualifiedName("java.rmi.server", "UnicastRemoteObject") and
68+
m.hasName("exportObject") and
69+
not m.getParameterType([2, 4]).(RefType).hasQualifiedName("java.io", "ObjectInputFilter") and
70+
ma.getArgument(0) = fromNode.asExpr() and
71+
ma = toNode.asExpr()
72+
)
73+
}
74+
}
75+
76+
from DataFlow::PathNode source, DataFlow::PathNode sink, BindingUnsafeRemoteObjectConfig conf
77+
where conf.hasFlowPath(source, sink)
78+
select sink.getNode(), source, sink, "Unsafe deserialization in a remote object."

‎java/ql/test/experimental/query-tests/security/CWE-502/UnsafeDeserializationRmi.expected

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
edges
2+
| UnsafeDeserializationRmi.java:17:68:17:95 | new UnsafeRemoteObjectImpl(...) : UnsafeRemoteObjectImpl | UnsafeDeserializationRmi.java:17:35:17:96 | exportObject(...) |
3+
nodes
4+
| UnsafeDeserializationRmi.java:15:33:15:60 | new UnsafeRemoteObjectImpl(...) | semmle.label | new UnsafeRemoteObjectImpl(...) |
5+
| UnsafeDeserializationRmi.java:16:35:16:62 | new UnsafeRemoteObjectImpl(...) | semmle.label | new UnsafeRemoteObjectImpl(...) |
6+
| UnsafeDeserializationRmi.java:17:35:17:96 | exportObject(...) | semmle.label | exportObject(...) |
7+
| UnsafeDeserializationRmi.java:17:68:17:95 | new UnsafeRemoteObjectImpl(...) : UnsafeRemoteObjectImpl | semmle.label | new UnsafeRemoteObjectImpl(...) : UnsafeRemoteObjectImpl |
8+
| UnsafeDeserializationRmi.java:29:31:29:58 | new UnsafeRemoteObjectImpl(...) | semmle.label | new UnsafeRemoteObjectImpl(...) |
9+
| UnsafeDeserializationRmi.java:30:33:30:60 | new UnsafeRemoteObjectImpl(...) | semmle.label | new UnsafeRemoteObjectImpl(...) |
10+
#select
11+
| 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. |
12+
| 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. |
13+
| 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. |
14+
| 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. |
15+
| 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. |

‎java/ql/test/experimental/query-tests/security/CWE-502/UnsafeDeserializationRmi.java

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
import java.io.ObjectInputFilter;
2+
import java.io.ObjectInputStream;
3+
import java.rmi.Naming;
4+
import java.rmi.Remote;
5+
import java.rmi.RemoteException;
6+
import java.rmi.registry.LocateRegistry;
7+
import java.rmi.registry.Registry;
8+
import java.rmi.server.UnicastRemoteObject;
9+
10+
public class UnsafeDeserializationRmi {
11+
12+
// BAD (bind a remote object that has a vulnerable method)
13+
public static void testRegistryBindWithObjectParameter() throws Exception {
14+
Registry registry = LocateRegistry.createRegistry(1099);
15+
registry.bind("unsafe", new UnsafeRemoteObjectImpl());
16+
registry.rebind("unsafe", new UnsafeRemoteObjectImpl());
17+
registry.rebind("unsafe", UnicastRemoteObject.exportObject(new UnsafeRemoteObjectImpl()));
18+
}
19+
20+
// GOOD (bind a remote object that has methods that takes safe parameters)
21+
public static void testRegistryBindWithIntParameter() throws Exception {
22+
Registry registry = LocateRegistry.createRegistry(1099);
23+
registry.bind("safe", new SafeRemoteObjectImpl());
24+
registry.rebind("safe", new SafeRemoteObjectImpl());
25+
}
26+
27+
// BAD (bind a remote object that has a vulnerable method)
28+
public static void testNamingBindWithObjectParameter() throws Exception {
29+
Naming.bind("unsafe", new UnsafeRemoteObjectImpl());
30+
Naming.rebind("unsafe", new UnsafeRemoteObjectImpl());
31+
}
32+
33+
// GOOD (bind a remote object that has methods that takes safe parameters)
34+
public static void testNamingBindWithIntParameter() throws Exception {
35+
Naming.bind("safe", new SafeRemoteObjectImpl());
36+
Naming.rebind("safe", new SafeRemoteObjectImpl());
37+
}
38+
39+
// GOOD (bind a remote object with a deserialization filter)
40+
public static void testRegistryBindWithDeserializationFilter() throws Exception {
41+
Registry registry = LocateRegistry.createRegistry(1099);
42+
ObjectInputFilter filter = info -> {
43+
if (info.serialClass().getCanonicalName().startsWith("com.safe.package.")) {
44+
return ObjectInputFilter.Status.ALLOWED;
45+
}
46+
return ObjectInputFilter.Status.REJECTED;
47+
};
48+
registry.rebind("safe", UnicastRemoteObject.exportObject(new UnsafeRemoteObjectImpl(), 12345, filter));
49+
}
50+
}
51+
52+
interface UnsafeRemoteObject extends Remote {
53+
void take(Object obj) throws RemoteException;
54+
}
55+
56+
class UnsafeRemoteObjectImpl implements UnsafeRemoteObject {
57+
public void take(Object obj) throws RemoteException {}
58+
}
59+
60+
interface SafeRemoteObject extends Remote {
61+
void take(int n) throws RemoteException;
62+
void take(double n) throws RemoteException;
63+
void take(String s) throws RemoteException;
64+
void take(ObjectInputStream ois) throws RemoteException;
65+
}
66+
67+
class SafeRemoteObjectImpl implements SafeRemoteObject {
68+
public void take(int n) throws RemoteException {}
69+
public void take(double n) throws RemoteException {}
70+
public void take(String s) throws RemoteException {}
71+
public void take(ObjectInputStream ois) throws RemoteException {}
72+
public void safeMethod(Object object) {} // this method is not declared in SafeRemoteObject
73+
}

‎java/ql/test/experimental/query-tests/security/CWE-502/UnsafeDeserializationRmi.qlref

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

0 commit comments

Comments
 (0)