Skip to content

Commit 9b0e3b1

Browse files
aschackmullaschackmull
authored
Merge pull request github#5814 from JLLeitschuh/feat/JLL/jackson_as_taint_step
[Java] Add taint tracking through Jackson deserialization
2 parents ae6326b + 77c93dc commit 9b0e3b1

File tree

10 files changed

+267
-66
lines changed

10 files changed

+267
-66
lines changed

‎java/change-notes/2021-05-03-jackson-dataflow-deserialization.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
lgtm,codescanning
2+
* Increase coverage of dataflow through Jackson JSON deserialized objects.

‎java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ private module Frameworks {
7777
private import semmle.code.java.frameworks.ApacheHttp
7878
private import semmle.code.java.frameworks.apache.Lang
7979
private import semmle.code.java.frameworks.guava.Guava
80+
private import semmle.code.java.frameworks.jackson.JacksonSerializability
8081
private import semmle.code.java.security.ResponseSplitting
8182
private import semmle.code.java.security.XSS
8283
private import semmle.code.java.security.LdapInjection

‎java/ql/src/semmle/code/java/frameworks/jackson/JacksonSerializability.qll

Lines changed: 51 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import semmle.code.java.Reflection
99
import semmle.code.java.dataflow.DataFlow
1010
import semmle.code.java.dataflow.DataFlow5
1111
import semmle.code.java.dataflow.FlowSteps
12+
private import semmle.code.java.dataflow.ExternalFlow
1213

1314
/**
1415
* A `@com.fasterxml.jackson.annotation.JsonIgnore` annoation.
@@ -28,7 +29,7 @@ abstract class JacksonSerializableType extends Type { }
2829
* A method used for serializing objects using Jackson. The final parameter is the object to be
2930
* serialized.
3031
*/
31-
library class JacksonWriteValueMethod extends Method, TaintPreservingCallable {
32+
private class JacksonWriteValueMethod extends Method, TaintPreservingCallable {
3233
JacksonWriteValueMethod() {
3334
(
3435
getDeclaringType().hasQualifiedName("com.fasterxml.jackson.databind", "ObjectWriter") or
@@ -50,8 +51,20 @@ library class JacksonWriteValueMethod extends Method, TaintPreservingCallable {
5051
}
5152
}
5253

54+
private class JacksonReadValueMethod extends Method, TaintPreservingCallable {
55+
JacksonReadValueMethod() {
56+
(
57+
getDeclaringType().hasQualifiedName("com.fasterxml.jackson.databind", "ObjectReader") or
58+
getDeclaringType().hasQualifiedName("com.fasterxml.jackson.databind", "ObjectMapper")
59+
) and
60+
hasName(["readValue", "readValues"])
61+
}
62+
63+
override predicate returnsTaintFrom(int arg) { arg = 0 }
64+
}
65+
5366
/** A type whose values are explicitly serialized in a call to a Jackson method. */
54-
library class ExplicitlyWrittenJacksonSerializableType extends JacksonSerializableType {
67+
private class ExplicitlyWrittenJacksonSerializableType extends JacksonSerializableType {
5568
ExplicitlyWrittenJacksonSerializableType() {
5669
exists(MethodAccess ma |
5770
// A call to a Jackson write method...
@@ -63,7 +76,7 @@ library class ExplicitlyWrittenJacksonSerializableType extends JacksonSerializab
6376
}
6477

6578
/** A type used in a `JacksonSerializableField` declaration. */
66-
library class FieldReferencedJacksonSerializableType extends JacksonSerializableType {
79+
private class FieldReferencedJacksonSerializableType extends JacksonSerializableType {
6780
FieldReferencedJacksonSerializableType() {
6881
exists(JacksonSerializableField f | usesType(f.getType(), this))
6982
}
@@ -96,17 +109,24 @@ private class TypeLiteralToJacksonDatabindFlowConfiguration extends DataFlow5::C
96109
}
97110

98111
/** A type whose values are explicitly deserialized in a call to a Jackson method. */
99-
library class ExplicitlyReadJacksonDeserializableType extends JacksonDeserializableType {
112+
private class ExplicitlyReadJacksonDeserializableType extends JacksonDeserializableType {
100113
ExplicitlyReadJacksonDeserializableType() {
101114
exists(TypeLiteralToJacksonDatabindFlowConfiguration conf |
102115
usesType(conf.getSourceWithFlowToJacksonDatabind().getTypeName().getType(), this)
103116
)
117+
or
118+
exists(MethodAccess ma |
119+
// A call to a Jackson read method...
120+
ma.getMethod() instanceof JacksonReadValueMethod and
121+
// ...where `this` is used in the final argument, indicating that this type will be deserialized.
122+
usesType(ma.getArgument(ma.getNumArgument() - 1).getType(), this)
123+
)
104124
}
105125
}
106126

107127
/** A type used in a `JacksonDeserializableField` declaration. */
108-
library class FieldReferencedJacksonDeSerializableType extends JacksonDeserializableType {
109-
FieldReferencedJacksonDeSerializableType() {
128+
private class FieldReferencedJacksonDeserializableType extends JacksonDeserializableType {
129+
FieldReferencedJacksonDeserializableType() {
110130
exists(JacksonDeserializableField f | usesType(f.getType(), this))
111131
}
112132
}
@@ -135,6 +155,21 @@ class JacksonDeserializableField extends DeserializableField {
135155
}
136156
}
137157

158+
/** A call to a field that may be deserialized using the Jackson JSON framework. */
159+
private class JacksonDeserializableFieldAccess extends FieldAccess {
160+
JacksonDeserializableFieldAccess() { getField() instanceof JacksonDeserializableField }
161+
}
162+
163+
/**
164+
* When an object is deserialized by the Jackson JSON framework using a tainted input source,
165+
* the fields that the framework deserialized are themselves tainted input data.
166+
*/
167+
private class JacksonDeserializedTaintStep extends AdditionalTaintStep {
168+
override predicate step(DataFlow::Node node1, DataFlow::Node node2) {
169+
DataFlow::getFieldQualifier(node2.asExpr().(JacksonDeserializableFieldAccess)) = node1
170+
}
171+
}
172+
138173
/**
139174
* A call to the `addMixInAnnotations` or `addMixIn` Jackson method.
140175
*
@@ -239,3 +274,13 @@ class JacksonMixedInCallable extends Callable {
239274
)
240275
}
241276
}
277+
278+
private class JacksonModel extends SummaryModelCsv {
279+
override predicate row(string row) {
280+
row =
281+
[
282+
"com.fasterxml.jackson.databind;ObjectMapper;true;valueToTree;;;Argument[0];ReturnValue;taint",
283+
"com.fasterxml.jackson.databind;ObjectMapper;true;convertValue;;;Argument[0];ReturnValue;taint"
284+
]
285+
}
286+
}

‎java/ql/test/library-tests/dataflow/taint-jackson/Test.java

Lines changed: 64 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,50 +3,110 @@
33
import java.io.OutputStream;
44
import java.io.StringWriter;
55
import java.io.Writer;
6+
import java.util.Iterator;
7+
import java.util.HashMap;
8+
import java.util.Map;
69

710
import com.fasterxml.jackson.core.JsonFactory;
811
import com.fasterxml.jackson.core.JsonGenerator;
12+
import com.fasterxml.jackson.databind.JsonNode;
913
import com.fasterxml.jackson.databind.ObjectMapper;
1014
import com.fasterxml.jackson.databind.ObjectWriter;
15+
import com.fasterxml.jackson.databind.ObjectReader;
1116

1217
class Test {
18+
public static class Potato {
19+
private String name;
20+
21+
private String getName() {
22+
return name;
23+
}
24+
}
25+
1326
public static String taint() {
1427
return "tainted";
1528
}
1629

30+
public static void sink(Object any) {}
31+
1732
public static void jacksonObjectMapper() throws java.io.FileNotFoundException, java.io.UnsupportedEncodingException {
1833
String s = taint();
1934
ObjectMapper om = new ObjectMapper();
2035
File file = new File("testFile");
2136
om.writeValue(file, s);
37+
sink(file); //$hasTaintFlow
2238
OutputStream out = new FileOutputStream(file);
2339
om.writeValue(out, s);
40+
sink(file); //$hasTaintFlow
2441
Writer writer = new StringWriter();
2542
om.writeValue(writer, s);
43+
sink(writer); //$hasTaintFlow
2644
JsonGenerator generator = new JsonFactory().createGenerator(new StringWriter());
2745
om.writeValue(generator, s);
46+
sink(generator); //$hasTaintFlow
2847
String t = om.writeValueAsString(s);
29-
System.out.println(t);
48+
sink(t); //$hasTaintFlow
3049
byte[] bs = om.writeValueAsBytes(s);
3150
String reconstructed = new String(bs, "utf-8");
32-
System.out.println(reconstructed);
51+
sink(bs); //$hasTaintFlow
52+
sink(reconstructed); //$hasTaintFlow
3353
}
3454

3555
public static void jacksonObjectWriter() throws java.io.FileNotFoundException, java.io.UnsupportedEncodingException {
3656
String s = taint();
3757
ObjectWriter ow = new ObjectWriter();
3858
File file = new File("testFile");
3959
ow.writeValue(file, s);
60+
sink(file); //$hasTaintFlow
4061
OutputStream out = new FileOutputStream(file);
4162
ow.writeValue(out, s);
63+
sink(out); //$hasTaintFlow
4264
Writer writer = new StringWriter();
4365
ow.writeValue(writer, s);
66+
sink(writer); //$hasTaintFlow
4467
JsonGenerator generator = new JsonFactory().createGenerator(new StringWriter());
4568
ow.writeValue(generator, s);
69+
sink(generator); //$hasTaintFlow
4670
String t = ow.writeValueAsString(s);
47-
System.out.println(t);
71+
sink(t); //$hasTaintFlow
4872
byte[] bs = ow.writeValueAsBytes(s);
4973
String reconstructed = new String(bs, "utf-8");
50-
System.out.println(reconstructed);
74+
sink(bs); //$hasTaintFlow
75+
sink(reconstructed); //$hasTaintFlow
76+
}
77+
78+
public static void jacksonObjectReader() throws java.io.IOException {
79+
String s = taint();
80+
ObjectMapper om = new ObjectMapper();
81+
ObjectReader reader = om.readerFor(Potato.class);
82+
sink(reader.readValue(s)); //$hasTaintFlow
83+
sink(reader.readValue(s, Potato.class).name); //$hasTaintFlow
84+
sink(reader.readValue(s, Potato.class).getName()); //$hasTaintFlow
85+
}
86+
87+
public static void jacksonObjectReaderIterable() throws java.io.IOException {
88+
String s = taint();
89+
ObjectMapper om = new ObjectMapper();
90+
ObjectReader reader = om.readerFor(Potato.class);
91+
sink(reader.readValues(s)); //$hasTaintFlow
92+
Iterator<Potato> pIterator = reader.readValues(s, Potato.class);
93+
while(pIterator.hasNext()) {
94+
Potato p = pIterator.next();
95+
sink(p); //$hasTaintFlow
96+
sink(p.name); //$hasTaintFlow
97+
sink(p.getName()); //$hasTaintFlow
98+
}
99+
}
100+
101+
public static void jacksonTwoStepDeserialization() throws java.io.IOException {
102+
String s = taint();
103+
Map<String, Object> taintedParams = new HashMap<>();
104+
taintedParams.put("name", s);
105+
ObjectMapper om = new ObjectMapper();
106+
JsonNode jn = om.valueToTree(taintedParams);
107+
sink(jn); //$hasTaintFlow
108+
Potato p = om.convertValue(jn, Potato.class);
109+
sink(p); //$hasTaintFlow
110+
sink(p.getName()); //$hasTaintFlow
51111
}
52112
}

‎java/ql/test/library-tests/dataflow/taint-jackson/dataFlow.expected

Lines changed: 0 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -1,48 +0,0 @@
1-
| ../../../stubs/jackson-databind-2.10/com/fasterxml/jackson/databind/ObjectMapper.java:10:43:10:54 | value |
2-
| ../../../stubs/jackson-databind-2.10/com/fasterxml/jackson/databind/ObjectMapper.java:13:73:13:84 | value |
3-
| ../../../stubs/jackson-databind-2.10/com/fasterxml/jackson/databind/ObjectMapper.java:16:44:16:55 | value |
4-
| ../../../stubs/jackson-databind-2.10/com/fasterxml/jackson/databind/ObjectMapper.java:19:36:19:47 | value |
5-
| ../../../stubs/jackson-databind-2.10/com/fasterxml/jackson/databind/ObjectMapper.java:22:35:22:46 | value |
6-
| ../../../stubs/jackson-databind-2.10/com/fasterxml/jackson/databind/ObjectMapper.java:26:36:26:47 | value |
7-
| ../../../stubs/jackson-databind-2.10/com/fasterxml/jackson/databind/ObjectWriter.java:10:43:10:54 | value |
8-
| ../../../stubs/jackson-databind-2.10/com/fasterxml/jackson/databind/ObjectWriter.java:13:73:13:84 | value |
9-
| ../../../stubs/jackson-databind-2.10/com/fasterxml/jackson/databind/ObjectWriter.java:16:44:16:55 | value |
10-
| ../../../stubs/jackson-databind-2.10/com/fasterxml/jackson/databind/ObjectWriter.java:19:36:19:47 | value |
11-
| ../../../stubs/jackson-databind-2.10/com/fasterxml/jackson/databind/ObjectWriter.java:22:35:22:46 | value |
12-
| ../../../stubs/jackson-databind-2.10/com/fasterxml/jackson/databind/ObjectWriter.java:26:36:26:47 | value |
13-
| Test.java:18:14:18:20 | taint(...) |
14-
| Test.java:21:17:21:20 | file [post update] |
15-
| Test.java:21:23:21:23 | s |
16-
| Test.java:22:43:22:46 | file |
17-
| Test.java:23:17:23:19 | out [post update] |
18-
| Test.java:23:22:23:22 | s |
19-
| Test.java:25:17:25:22 | writer [post update] |
20-
| Test.java:25:25:25:25 | s |
21-
| Test.java:27:17:27:25 | generator [post update] |
22-
| Test.java:27:28:27:28 | s |
23-
| Test.java:28:14:28:37 | writeValueAsString(...) |
24-
| Test.java:28:36:28:36 | s |
25-
| Test.java:29:22:29:22 | t |
26-
| Test.java:30:15:30:37 | writeValueAsBytes(...) |
27-
| Test.java:30:36:30:36 | s |
28-
| Test.java:31:26:31:48 | new String(...) |
29-
| Test.java:31:37:31:38 | bs |
30-
| Test.java:32:22:32:34 | reconstructed |
31-
| Test.java:36:14:36:20 | taint(...) |
32-
| Test.java:39:17:39:20 | file [post update] |
33-
| Test.java:39:23:39:23 | s |
34-
| Test.java:40:43:40:46 | file |
35-
| Test.java:41:17:41:19 | out [post update] |
36-
| Test.java:41:22:41:22 | s |
37-
| Test.java:43:17:43:22 | writer [post update] |
38-
| Test.java:43:25:43:25 | s |
39-
| Test.java:45:17:45:25 | generator [post update] |
40-
| Test.java:45:28:45:28 | s |
41-
| Test.java:46:14:46:37 | writeValueAsString(...) |
42-
| Test.java:46:36:46:36 | s |
43-
| Test.java:47:22:47:22 | t |
44-
| Test.java:48:15:48:37 | writeValueAsBytes(...) |
45-
| Test.java:48:36:48:36 | s |
46-
| Test.java:49:26:49:48 | new String(...) |
47-
| Test.java:49:37:49:38 | bs |
48-
| Test.java:50:22:50:34 | reconstructed |

‎java/ql/test/library-tests/dataflow/taint-jackson/dataFlow.ql

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,34 @@
1+
import java
12
import semmle.code.java.dataflow.DataFlow
23
import semmle.code.java.dataflow.TaintTracking
34
import semmle.code.java.dataflow.FlowSources
5+
import TestUtilities.InlineExpectationsTest
46

57
class Conf extends TaintTracking::Configuration {
68
Conf() { this = "qltest:dataflow:jackson" }
79

8-
override predicate isSource(DataFlow::Node source) {
9-
source.asExpr().(MethodAccess).getMethod().hasName("taint")
10+
override predicate isSource(DataFlow::Node n) {
11+
n.asExpr().(MethodAccess).getMethod().hasName("taint")
12+
or
13+
n instanceof RemoteFlowSource
1014
}
1115

12-
override predicate isSink(DataFlow::Node sink) { any() }
16+
override predicate isSink(DataFlow::Node n) {
17+
exists(MethodAccess ma | ma.getMethod().hasName("sink") | n.asExpr() = ma.getAnArgument())
18+
}
1319
}
1420

15-
from DataFlow::Node source, DataFlow::Node sink, Conf config
16-
where config.hasFlow(source, sink)
17-
select sink
21+
class HasFlowTest extends InlineExpectationsTest {
22+
HasFlowTest() { this = "HasFlowTest" }
23+
24+
override string getARelevantTag() { result = "hasTaintFlow" }
25+
26+
override predicate hasActualResult(Location location, string element, string tag, string value) {
27+
tag = "hasTaintFlow" and
28+
exists(DataFlow::Node src, DataFlow::Node sink, Conf conf | conf.hasFlow(src, sink) |
29+
sink.getLocation() = location and
30+
element = sink.toString() and
31+
value = ""
32+
)
33+
}
34+
}

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

Lines changed: 4 additions & 2 deletions
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 java.util.*;
4+
5+
public abstract class JsonNode implements Iterable<JsonNode> {
46
public JsonNode() {
57
}
6-
}
8+
}

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
package com.fasterxml.jackson.databind;
2+
3+
import java.io.Closeable;
4+
import java.io.IOException;
5+
import java.util.*;
6+
7+
public class MappingIterator<T> implements Iterator<T>, Closeable {
8+
9+
@Override
10+
public boolean hasNext() {
11+
return false;
12+
}
13+
14+
@Override
15+
public T next() {
16+
return null;
17+
}
18+
19+
@Override
20+
public void remove() {
21+
22+
}
23+
24+
@Override
25+
public void close() throws IOException {
26+
27+
}
28+
}

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,4 +26,16 @@ public byte[] writeValueAsBytes(Object value) {
2626
public String writeValueAsString(Object value) {
2727
return null;
2828
}
29+
30+
public ObjectReader readerFor(Class<?> type) {
31+
return null;
32+
}
33+
34+
public <T extends JsonNode> T valueToTree(Object fromValue) throws IllegalArgumentException {
35+
return null;
36+
}
37+
38+
public <T> T convertValue(Object fromValue, Class<T> toValueType) throws IllegalArgumentException {
39+
return null;
40+
}
2941
}

0 commit comments

Comments
 (0)