Skip to content

Commit 2faaa75

Browse files
artem-smotrakovartem-smotrakov
committed
Added tests for UnsafeDeserialization.ql and Jackson
1 parent d0f3524 commit 2faaa75

File tree

15 files changed

+339
-2
lines changed

15 files changed

+339
-2
lines changed

‎java/ql/src/semmle/code/java/security/UnsafeDeserialization.qll

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,11 +269,19 @@ predicate createJacksonTreeNodeStep(DataFlow::Node fromNode, DataFlow::Node toNo
269269
)
270270
}
271271

272+
/**
273+
* Holds if `type` or one of its supertypes has a field with `JsonTypeInfo` annotation
274+
* that enables polymorphic type handling.
275+
*/
272276
predicate hasJsonTypeInfoAnnotation(RefType type) {
273277
hasFieldWithJsonTypeAnnotation(type.getASupertype*()) or
274278
hasFieldWithJsonTypeAnnotation(type.getAField().getType())
275279
}
276280

281+
/**
282+
* Holds if `type` has a field with `JsonTypeInfo` annotation
283+
* that enables polymorphic type handling.
284+
*/
277285
predicate hasFieldWithJsonTypeAnnotation(RefType type) {
278286
exists(Annotation a |
279287
type.getAField().getAnAnnotation() = a and

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

Lines changed: 193 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,193 @@
1+
import com.fasterxml.jackson.annotation.JsonTypeInfo;
2+
import com.fasterxml.jackson.core.JsonFactory;
3+
import com.fasterxml.jackson.databind.ObjectMapper;
4+
import com.fasterxml.jackson.databind.json.JsonMapper;
5+
import com.fasterxml.jackson.databind.jsontype.BasicPolymorphicTypeValidator;
6+
import com.fasterxml.jackson.databind.jsontype.PolymorphicTypeValidator;
7+
import java.io.IOException;
8+
import java.io.Serializable;
9+
import java.net.ServerSocket;
10+
import java.net.Socket;
11+
import java.util.List;
12+
13+
public class JacksonTest {
14+
15+
public static void withSocket(Action<String> action) throws Exception {
16+
try (ServerSocket serverSocket = new ServerSocket(0)) {
17+
try (Socket socket = serverSocket.accept()) {
18+
byte[] bytes = new byte[1024];
19+
int n = socket.getInputStream().read(bytes);
20+
String jexlExpr = new String(bytes, 0, n);
21+
action.run(jexlExpr);
22+
}
23+
}
24+
}
25+
}
26+
27+
interface Action<T> {
28+
void run(T object) throws Exception;
29+
}
30+
31+
abstract class PhoneNumber implements Serializable {
32+
public int areaCode;
33+
public int local;
34+
}
35+
36+
class DomesticNumber extends PhoneNumber {
37+
}
38+
39+
class InternationalNumber extends PhoneNumber {
40+
public int countryCode;
41+
}
42+
43+
class Employee extends Person {
44+
}
45+
46+
class Person {
47+
public String name;
48+
public int age;
49+
50+
// this annotation enables polymorphic type handling
51+
@JsonTypeInfo(use = JsonTypeInfo.Id.CLASS)
52+
public Object phone;
53+
}
54+
55+
class Task {
56+
public Person assignee;
57+
}
58+
59+
class Tag implements Serializable {
60+
public String title;
61+
}
62+
63+
class Cat {
64+
public String name;
65+
public Serializable tag;
66+
}
67+
68+
class UnsafePersonDeserialization {
69+
70+
// BAD: Person has a field with an annotation that enables polymorphic type
71+
// handling
72+
private static void testUnsafeDeserialization() throws Exception {
73+
JacksonTest.withSocket(string -> {
74+
ObjectMapper mapper = new ObjectMapper();
75+
mapper.readValue(string, Person.class);
76+
});
77+
}
78+
79+
// BAD: Employee extends Person that has a field with an annotation that enables
80+
// polymorphic type handling
81+
private static void testUnsafeDeserializationWithExtendedClass() throws Exception {
82+
JacksonTest.withSocket(string -> {
83+
ObjectMapper mapper = new ObjectMapper();
84+
mapper.readValue(string, Employee.class);
85+
});
86+
}
87+
88+
// BAD: Task has a Person field that has a field with an annotation that enables
89+
// polymorphic type handling
90+
private static void testUnsafeDeserializationWithWrapper() throws Exception {
91+
JacksonTest.withSocket(string -> {
92+
ObjectMapper mapper = new ObjectMapper();
93+
mapper.readValue(string, Task.class);
94+
});
95+
}
96+
}
97+
98+
class SaferPersonDeserialization {
99+
100+
// GOOD: Despite enabled polymorphic type handling, this is safe because ObjectMapper
101+
// has a validator
102+
private static void testSafeDeserializationWithValidator() throws Exception {
103+
JacksonTest.withSocket(string -> {
104+
PolymorphicTypeValidator ptv =
105+
BasicPolymorphicTypeValidator.builder()
106+
.allowIfSubType("only.allowed.package")
107+
.build();
108+
109+
ObjectMapper mapper = new ObjectMapper();
110+
mapper.setPolymorphicTypeValidator(ptv);
111+
112+
mapper.readValue(string, Person.class);
113+
});
114+
}
115+
116+
// GOOD: Despite enabled polymorphic type handling, this is safe because ObjectMapper
117+
// has a validator
118+
private static void testSafeDeserializationWithValidatorAndBuilder() throws Exception {
119+
JacksonTest.withSocket(string -> {
120+
PolymorphicTypeValidator ptv =
121+
BasicPolymorphicTypeValidator.builder()
122+
.allowIfSubType("only.allowed.package")
123+
.build();
124+
125+
ObjectMapper mapper = JsonMapper.builder()
126+
.polymorphicTypeValidator(ptv)
127+
.build();
128+
129+
mapper.readValue(string, Person.class);
130+
});
131+
}
132+
}
133+
134+
class UnsafeCatDeserialization {
135+
136+
// BAD: deserializing untrusted input while polymorphic type handling is on
137+
private static void testUnsafeDeserialization() throws Exception {
138+
JacksonTest.withSocket(string -> {
139+
ObjectMapper mapper = new ObjectMapper();
140+
mapper.enableDefaultTyping(); // this enables polymorphic type handling
141+
mapper.readValue(string, Cat.class);
142+
});
143+
}
144+
145+
// BAD: deserializing untrusted input while polymorphic type handling is on
146+
private static void testUnsafeDeserializationWithObjectMapperReadValues() throws Exception {
147+
JacksonTest.withSocket(string -> {
148+
ObjectMapper mapper = new ObjectMapper();
149+
mapper.enableDefaultTyping();
150+
mapper.readValues(new JsonFactory().createParser(string), Cat.class).readAll();
151+
});
152+
}
153+
154+
// BAD: deserializing untrusted input while polymorphic type handling is on
155+
private static void testUnsafeDeserializationWithObjectMapperTreeToValue() throws Exception {
156+
JacksonTest.withSocket(string -> {
157+
ObjectMapper mapper = new ObjectMapper();
158+
mapper.enableDefaultTyping();
159+
mapper.treeToValue(mapper.readTree(string), Cat.class);
160+
});
161+
}
162+
163+
// BAD: an attacker can control both data and type of deserialized object
164+
private static void testUnsafeDeserializationWithUnsafeClass() throws Exception {
165+
JacksonTest.withSocket(input -> {
166+
String[] parts = input.split(";");
167+
String data = parts[0];
168+
String type = parts[1];
169+
Class clazz = Class.forName(type);
170+
ObjectMapper mapper = new ObjectMapper();
171+
mapper.readValue(data, clazz);
172+
});
173+
}
174+
}
175+
176+
class SaferCatDeserialization {
177+
178+
// GOOD: Despite enabled polymorphic type handling, this is safe because ObjectMapper
179+
// has a validator
180+
private static void testUnsafeDeserialization() throws Exception {
181+
JacksonTest.withSocket(string -> {
182+
PolymorphicTypeValidator ptv =
183+
BasicPolymorphicTypeValidator.builder()
184+
.allowIfSubType("only.allowed.pachage")
185+
.build();
186+
187+
ObjectMapper mapper = JsonMapper.builder().polymorphicTypeValidator(ptv).build();
188+
mapper.enableDefaultTyping(); // this enables polymorphic type handling
189+
190+
mapper.readValue(string, Cat.class);
191+
});
192+
}
193+
}

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

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,22 @@ edges
4848
| B.java:27:31:27:51 | getInputStream(...) : InputStream | B.java:29:5:29:15 | inputStream : InputStream |
4949
| B.java:29:5:29:15 | inputStream : InputStream | B.java:29:22:29:26 | bytes [post update] : byte[] |
5050
| B.java:29:22:29:26 | bytes [post update] : byte[] | B.java:31:23:31:23 | s |
51+
| JacksonTest.java:19:25:19:47 | getInputStream(...) : InputStream | JacksonTest.java:19:54:19:58 | bytes [post update] : byte[] |
52+
| JacksonTest.java:19:54:19:58 | bytes [post update] : byte[] | JacksonTest.java:21:28:21:35 | jexlExpr : String |
53+
| JacksonTest.java:21:28:21:35 | jexlExpr : String | JacksonTest.java:73:32:73:37 | string : String |
54+
| JacksonTest.java:21:28:21:35 | jexlExpr : String | JacksonTest.java:82:32:82:37 | string : String |
55+
| JacksonTest.java:21:28:21:35 | jexlExpr : String | JacksonTest.java:91:32:91:37 | string : String |
56+
| JacksonTest.java:21:28:21:35 | jexlExpr : String | JacksonTest.java:138:32:138:37 | string : String |
57+
| JacksonTest.java:21:28:21:35 | jexlExpr : String | JacksonTest.java:147:32:147:37 | string : String |
58+
| JacksonTest.java:21:28:21:35 | jexlExpr : String | JacksonTest.java:156:32:156:37 | string : String |
59+
| JacksonTest.java:21:28:21:35 | jexlExpr : String | JacksonTest.java:165:32:165:36 | input : String |
60+
| JacksonTest.java:73:32:73:37 | string : String | JacksonTest.java:75:30:75:35 | string |
61+
| JacksonTest.java:82:32:82:37 | string : String | JacksonTest.java:84:30:84:35 | string |
62+
| JacksonTest.java:91:32:91:37 | string : String | JacksonTest.java:93:30:93:35 | string |
63+
| JacksonTest.java:138:32:138:37 | string : String | JacksonTest.java:141:30:141:35 | string |
64+
| JacksonTest.java:147:32:147:37 | string : String | JacksonTest.java:150:31:150:68 | createParser(...) |
65+
| JacksonTest.java:156:32:156:37 | string : String | JacksonTest.java:159:32:159:54 | readTree(...) |
66+
| JacksonTest.java:165:32:165:36 | input : String | JacksonTest.java:171:30:171:33 | data |
5167
| TestMessageBodyReader.java:20:55:20:78 | entityStream : InputStream | TestMessageBodyReader.java:22:18:22:52 | new ObjectInputStream(...) |
5268
| TestMessageBodyReader.java:20:55:20:78 | entityStream : InputStream | TestMessageBodyReader.java:22:40:22:51 | entityStream : InputStream |
5369
| TestMessageBodyReader.java:22:40:22:51 | entityStream : InputStream | TestMessageBodyReader.java:22:18:22:52 | new ObjectInputStream(...) |
@@ -111,6 +127,23 @@ nodes
111127
| B.java:29:5:29:15 | inputStream : InputStream | semmle.label | inputStream : InputStream |
112128
| B.java:29:22:29:26 | bytes [post update] : byte[] | semmle.label | bytes [post update] : byte[] |
113129
| B.java:31:23:31:23 | s | semmle.label | s |
130+
| JacksonTest.java:19:25:19:47 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
131+
| JacksonTest.java:19:54:19:58 | bytes [post update] : byte[] | semmle.label | bytes [post update] : byte[] |
132+
| JacksonTest.java:21:28:21:35 | jexlExpr : String | semmle.label | jexlExpr : String |
133+
| JacksonTest.java:73:32:73:37 | string : String | semmle.label | string : String |
134+
| JacksonTest.java:75:30:75:35 | string | semmle.label | string |
135+
| JacksonTest.java:82:32:82:37 | string : String | semmle.label | string : String |
136+
| JacksonTest.java:84:30:84:35 | string | semmle.label | string |
137+
| JacksonTest.java:91:32:91:37 | string : String | semmle.label | string : String |
138+
| JacksonTest.java:93:30:93:35 | string | semmle.label | string |
139+
| JacksonTest.java:138:32:138:37 | string : String | semmle.label | string : String |
140+
| JacksonTest.java:141:30:141:35 | string | semmle.label | string |
141+
| JacksonTest.java:147:32:147:37 | string : String | semmle.label | string : String |
142+
| JacksonTest.java:150:31:150:68 | createParser(...) | semmle.label | createParser(...) |
143+
| JacksonTest.java:156:32:156:37 | string : String | semmle.label | string : String |
144+
| JacksonTest.java:159:32:159:54 | readTree(...) | semmle.label | readTree(...) |
145+
| JacksonTest.java:165:32:165:36 | input : String | semmle.label | input : String |
146+
| JacksonTest.java:171:30:171:33 | data | semmle.label | data |
114147
| TestMessageBodyReader.java:20:55:20:78 | entityStream : InputStream | semmle.label | entityStream : InputStream |
115148
| TestMessageBodyReader.java:22:18:22:52 | new ObjectInputStream(...) | semmle.label | new ObjectInputStream(...) |
116149
| TestMessageBodyReader.java:22:40:22:51 | entityStream : InputStream | semmle.label | entityStream : InputStream |
@@ -141,4 +174,11 @@ nodes
141174
| B.java:15:12:15:28 | parse(...) | B.java:12:31:12:51 | getInputStream(...) : InputStream | B.java:15:23:15:27 | bytes | Unsafe deserialization of $@. | B.java:12:31:12:51 | getInputStream(...) | user input |
142175
| B.java:23:12:23:30 | parseObject(...) | B.java:19:31:19:51 | getInputStream(...) : InputStream | B.java:23:29:23:29 | s | Unsafe deserialization of $@. | B.java:19:31:19:51 | getInputStream(...) | user input |
143176
| B.java:31:12:31:24 | parse(...) | B.java:27:31:27:51 | getInputStream(...) : InputStream | B.java:31:23:31:23 | s | Unsafe deserialization of $@. | B.java:27:31:27:51 | getInputStream(...) | user input |
177+
| JacksonTest.java:75:13:75:50 | readValue(...) | JacksonTest.java:19:25:19:47 | getInputStream(...) : InputStream | JacksonTest.java:75:30:75:35 | string | Unsafe deserialization of $@. | JacksonTest.java:19:25:19:47 | getInputStream(...) | user input |
178+
| JacksonTest.java:84:13:84:52 | readValue(...) | JacksonTest.java:19:25:19:47 | getInputStream(...) : InputStream | JacksonTest.java:84:30:84:35 | string | Unsafe deserialization of $@. | JacksonTest.java:19:25:19:47 | getInputStream(...) | user input |
179+
| JacksonTest.java:93:13:93:48 | readValue(...) | JacksonTest.java:19:25:19:47 | getInputStream(...) : InputStream | JacksonTest.java:93:30:93:35 | string | Unsafe deserialization of $@. | JacksonTest.java:19:25:19:47 | getInputStream(...) | user input |
180+
| JacksonTest.java:141:13:141:47 | readValue(...) | JacksonTest.java:19:25:19:47 | getInputStream(...) : InputStream | JacksonTest.java:141:30:141:35 | string | Unsafe deserialization of $@. | JacksonTest.java:19:25:19:47 | getInputStream(...) | user input |
181+
| JacksonTest.java:150:13:150:80 | readValues(...) | JacksonTest.java:19:25:19:47 | getInputStream(...) : InputStream | JacksonTest.java:150:31:150:68 | createParser(...) | Unsafe deserialization of $@. | JacksonTest.java:19:25:19:47 | getInputStream(...) | user input |
182+
| JacksonTest.java:159:13:159:66 | treeToValue(...) | JacksonTest.java:19:25:19:47 | getInputStream(...) : InputStream | JacksonTest.java:159:32:159:54 | readTree(...) | Unsafe deserialization of $@. | JacksonTest.java:19:25:19:47 | getInputStream(...) | user input |
183+
| JacksonTest.java:171:13:171:41 | readValue(...) | JacksonTest.java:19:25:19:47 | getInputStream(...) : InputStream | JacksonTest.java:171:30:171:33 | data | Unsafe deserialization of $@. | JacksonTest.java:19:25:19:47 | getInputStream(...) | user input |
144184
| TestMessageBodyReader.java:22:18:22:65 | readObject(...) | TestMessageBodyReader.java:20:55:20:78 | entityStream : InputStream | TestMessageBodyReader.java:22:18:22:52 | new ObjectInputStream(...) | Unsafe deserialization of $@. | TestMessageBodyReader.java:20:55:20:78 | entityStream | user input |

‎java/ql/test/query-tests/security/CWE-502/options

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
//semmle-extractor-options: --javac-args -cp ${testdir}/../../../stubs/snakeyaml-1.21:${testdir}/../../../stubs/xstream-1.4.10:${testdir}/../../../stubs/kryo-4.0.2:${testdir}/../../../stubs/jsr311-api-1.1.1:${testdir}/../../../stubs/fastjson-1.2.74
1+
//semmle-extractor-options: --javac-args -cp ${testdir}/../../../stubs/snakeyaml-1.21:${testdir}/../../../stubs/xstream-1.4.10:${testdir}/../../../stubs/kryo-4.0.2:${testdir}/../../../stubs/jsr311-api-1.1.1:${testdir}/../../../stubs/fastjson-1.2.74:${testdir}/../../../stubs/jackson-databind-2.10

‎java/ql/test/stubs/jackson-databind-2.10/com/fasterxml/jackson/annotation/JsonTypeInfo.java

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
package com.fasterxml.jackson.annotation;
2+
3+
import java.lang.annotation.ElementType;
4+
import java.lang.annotation.Retention;
5+
import java.lang.annotation.RetentionPolicy;
6+
import java.lang.annotation.Target;
7+
8+
@Target({ElementType.ANNOTATION_TYPE, ElementType.TYPE, ElementType.FIELD, ElementType.METHOD, ElementType.PARAMETER})
9+
@Retention(RetentionPolicy.RUNTIME)
10+
public @interface JsonTypeInfo {
11+
JsonTypeInfo.Id use();
12+
13+
public static enum Id {
14+
CLASS("@class"),
15+
MINIMAL_CLASS("@c");
16+
17+
private final String _defaultPropertyName;
18+
19+
private Id(String defProp) {
20+
this._defaultPropertyName = defProp;
21+
}
22+
23+
public String getDefaultPropertyName() {
24+
return this._defaultPropertyName;
25+
}
26+
}
27+
}

‎java/ql/test/stubs/jackson-databind-2.10/com/fasterxml/jackson/core/JsonFactory.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,8 @@ public JsonFactory() {
99
public JsonGenerator createGenerator(Writer writer) {
1010
return new JsonGenerator();
1111
}
12+
13+
public JsonParser createParser(String content) {
14+
return null;
15+
}
1216
}

‎java/ql/test/stubs/jackson-databind-2.10/com/fasterxml/jackson/core/JsonParser.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
package com.fasterxml.jackson.core;
2+
3+
public abstract class JsonParser {}

‎java/ql/test/stubs/jackson-databind-2.10/com/fasterxml/jackson/core/TreeNode.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
package com.fasterxml.jackson.core;
2+
3+
public interface TreeNode {}

‎java/ql/test/stubs/jackson-databind-2.10/com/fasterxml/jackson/databind/JsonNode.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package com.fasterxml.jackson.databind;
22

3-
public class JsonNode {
3+
import com.fasterxml.jackson.core.TreeNode;
4+
5+
public class JsonNode implements TreeNode {
46
public JsonNode() {
57
}
68
}

‎java/ql/test/stubs/jackson-databind-2.10/com/fasterxml/jackson/databind/MappingIterator.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
package com.fasterxml.jackson.databind;
2+
3+
import java.util.Iterator;
4+
import java.util.List;
5+
6+
public class MappingIterator<T> implements Iterator<T> {
7+
public boolean hasNext() { return false; }
8+
public T next() { return null; }
9+
public void remove() {}
10+
public List<T> readAll() { return null; }
11+
}

0 commit comments

Comments
 (0)