Skip to content

Commit c837605

Browse files
artem-smotrakovartem-smotrakov
committed
Added test cases with sanitizers for UnsafeDeserializationRmi.ql
1 parent d2e29fc commit c837605

File tree

3 files changed

+50
-24
lines changed

3 files changed

+50
-24
lines changed

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/**
2-
* @name Unsafe remote object.
2+
* @name Unsafe deserialization in a remotely callable method.
33
* @description If a registered remote object has a method that accepts a complex object,
44
* an attacker can take advantage of the unsafe deserialization mechanism
55
* which is used to pass parameters in RMI.
@@ -31,7 +31,7 @@ private class BindMethod extends Method {
3131
}
3232

3333
/**
34-
* Holds if `type` has an unsafe remote method.
34+
* Holds if `type` has an vulnerable remote method.
3535
*/
3636
private predicate hasVulnerableMethod(RefType type) {
3737
exists(RemoteCallableMethod m, Type parameterType |
@@ -57,13 +57,13 @@ private class BindingUnsafeRemoteObjectConfig extends TaintTracking::Configurati
5757
}
5858

5959
override predicate isSink(DataFlow::Node sink) {
60-
exists(StaticMethodAccess ma | ma.getArgument(1) = sink.asExpr() |
60+
exists(MethodAccess ma | ma.getArgument(1) = sink.asExpr() |
6161
ma.getMethod() instanceof BindMethod
6262
)
6363
}
6464

6565
override predicate isAdditionalTaintStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
66-
exists(StaticMethodAccess ma, Method m | m = ma.getMethod() |
66+
exists(MethodAccess ma, Method m | m = ma.getMethod() |
6767
m.getDeclaringType().hasQualifiedName("java.rmi.server", "UnicastRemoteObject") and
6868
m.hasName("exportObject") and
6969
not ma.getArgument([2, 4])
@@ -79,4 +79,4 @@ private class BindingUnsafeRemoteObjectConfig extends TaintTracking::Configurati
7979

8080
from DataFlow::PathNode source, DataFlow::PathNode sink, BindingUnsafeRemoteObjectConfig conf
8181
where conf.hasFlowPath(source, sink)
82-
select sink.getNode(), source, sink, "Binding an unsafe remote object."
82+
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 & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,15 @@
1-
| RmiUnsafeDeserialization.java:13:9:13:59 | bind(...) | Unsafe deserialization with RMI in '$@' method | RmiUnsafeDeserialization.java:42:17:42:20 | take | take(Object) |
2-
| RmiUnsafeDeserialization.java:14:9:14:61 | rebind(...) | Unsafe deserialization with RMI in '$@' method | RmiUnsafeDeserialization.java:42:17:42:20 | take | take(Object) |
3-
| RmiUnsafeDeserialization.java:26:9:26:57 | bind(...) | Unsafe deserialization with RMI in '$@' method | RmiUnsafeDeserialization.java:42:17:42:20 | take | take(Object) |
4-
| RmiUnsafeDeserialization.java:27:9:27:59 | rebind(...) | Unsafe deserialization with RMI in '$@' method | RmiUnsafeDeserialization.java:42:17:42:20 | take | take(Object) |
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: 30 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,58 +1,73 @@
1+
import java.io.ObjectInputFilter;
12
import java.io.ObjectInputStream;
23
import java.rmi.Naming;
34
import java.rmi.Remote;
45
import java.rmi.RemoteException;
56
import java.rmi.registry.LocateRegistry;
67
import java.rmi.registry.Registry;
8+
import java.rmi.server.UnicastRemoteObject;
79

810
public class UnsafeDeserializationRmi {
911

10-
// BAD (bind a remote object that has a vulnerable method that takes Object)
12+
// BAD (bind a remote object that has a vulnerable method)
1113
public static void testRegistryBindWithObjectParameter() throws Exception {
1214
Registry registry = LocateRegistry.createRegistry(1099);
13-
registry.bind("test", new RemoteObjectWithObject());
14-
registry.rebind("test", new RemoteObjectWithObject());
15+
registry.bind("unsafe", new UnsafeRemoteObjectImpl());
16+
registry.rebind("unsafe", new UnsafeRemoteObjectImpl());
17+
registry.rebind("unsafe", UnicastRemoteObject.exportObject(new UnsafeRemoteObjectImpl()));
1518
}
1619

1720
// GOOD (bind a remote object that has methods that takes safe parameters)
1821
public static void testRegistryBindWithIntParameter() throws Exception {
1922
Registry registry = LocateRegistry.createRegistry(1099);
20-
registry.bind("test", new SafeRemoteObject());
21-
registry.rebind("test", new SafeRemoteObject());
23+
registry.bind("safe", new SafeRemoteObjectImpl());
24+
registry.rebind("safe", new SafeRemoteObjectImpl());
2225
}
2326

24-
// BAD (bind a remote object that has a vulnerable method that takes Object)
27+
// BAD (bind a remote object that has a vulnerable method)
2528
public static void testNamingBindWithObjectParameter() throws Exception {
26-
Naming.bind("test", new RemoteObjectWithObject());
27-
Naming.rebind("test", new RemoteObjectWithObject());
29+
Naming.bind("unsafe", new UnsafeRemoteObjectImpl());
30+
Naming.rebind("unsafe", new UnsafeRemoteObjectImpl());
2831
}
2932

3033
// GOOD (bind a remote object that has methods that takes safe parameters)
3134
public static void testNamingBindWithIntParameter() throws Exception {
32-
Naming.bind("test", new SafeRemoteObject());
33-
Naming.rebind("test", new SafeRemoteObject());
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));
3449
}
3550
}
3651

37-
interface RemoteObjectWithObjectInterface extends Remote {
52+
interface UnsafeRemoteObject extends Remote {
3853
void take(Object obj) throws RemoteException;
3954
}
4055

41-
class RemoteObjectWithObject implements RemoteObjectWithObjectInterface {
56+
class UnsafeRemoteObjectImpl implements UnsafeRemoteObject {
4257
public void take(Object obj) throws RemoteException {}
4358
}
4459

45-
interface SafeRemoteObjectInterface extends Remote {
60+
interface SafeRemoteObject extends Remote {
4661
void take(int n) throws RemoteException;
4762
void take(double n) throws RemoteException;
4863
void take(String s) throws RemoteException;
4964
void take(ObjectInputStream ois) throws RemoteException;
5065
}
5166

52-
class SafeRemoteObject implements SafeRemoteObjectInterface {
67+
class SafeRemoteObjectImpl implements SafeRemoteObject {
5368
public void take(int n) throws RemoteException {}
5469
public void take(double n) throws RemoteException {}
5570
public void take(String s) throws RemoteException {}
5671
public void take(ObjectInputStream ois) throws RemoteException {}
57-
public void safeMethod(Object object) {} // this method is not declared in SafeRemoteObjectInterface
72+
public void safeMethod(Object object) {} // this method is not declared in SafeRemoteObject
5873
}

0 commit comments

Comments
 (0)