Skip to content

Commit db2892b

Browse files
committed
Resove taint tracking issues from asMultimap
Signed-off-by: Jonathan Leitschuh <Jonathan.Leitschuh@gmail.com>
1 parent 5a2bdc9 commit db2892b

File tree

4 files changed

+98
-5
lines changed

4 files changed

+98
-5
lines changed

java/ql/lib/semmle/code/java/frameworks/ratpack/Ratpack.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ private class RatpackModel extends SummaryModelCsv {
8181
"MultiValueMap;true;getAll;();;MapValue of Argument[-1];Element of MapValue of ReturnValue;value",
8282
"MultiValueMap;true;getAll;(Object);;MapValue of Argument[-1];Element of ReturnValue;value",
8383
"MultiValueMap;true;asMultimap;;;MapKey of Argument[-1];MapKey of ReturnValue;value",
84-
"MultiValueMap;true;asMultimap;;;MapValue of Argument[-1];Element of MapValue of ReturnValue;value"
84+
"MultiValueMap;true;asMultimap;;;MapValue of Argument[-1];MapValue of ReturnValue;value"
8585
]
8686
}
8787
}
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
import com.google.common.collect.ImmutableList;
2+
import com.google.common.collect.ImmutableMap;
3+
4+
import ratpack.core.handling.Context;
5+
import ratpack.core.form.Form;
6+
7+
import java.util.Collection;
8+
import java.util.HashMap;
9+
import java.util.Map;
10+
import java.util.List;
11+
import java.util.function.Predicate;
12+
13+
public class CollectionPassingTest {
14+
15+
void sink(Object o) {}
16+
17+
String taint() {
18+
return null;
19+
}
20+
21+
void test_1(Context ctx) {
22+
// Given
23+
ctx
24+
.getRequest()
25+
.getBody()
26+
.map(data -> ctx.parse(data, Form.form()))
27+
.then(form -> {
28+
// When
29+
Map<String, Object> pojoMap = new HashMap<>();
30+
merge(form.asMultimap().asMap(), pojoMap);
31+
// Then
32+
sink(pojoMap.get("value")); //$hasTaintFlow
33+
pojoMap.forEach((key, value) -> {
34+
sink(value); //$hasTaintFlow
35+
List<Object> values = (List<Object>) value;
36+
sink(values.get(0)); //$hasTaintFlow
37+
});
38+
});
39+
}
40+
41+
void test_2() {
42+
// Given
43+
Map<String, Collection<String>> taintedMap = new HashMap<>();
44+
taintedMap.put("value", ImmutableList.of(taint()));
45+
Map<String, Object> pojoMap = new HashMap<>();
46+
// When
47+
merge(taintedMap, pojoMap);
48+
// Then
49+
sink(pojoMap.get("value")); //$hasTaintFlow
50+
pojoMap.forEach((key, value) -> {
51+
sink(value); //$hasTaintFlow
52+
List<Object> values = (List<Object>) value;
53+
sink(values.get(0)); //$hasTaintFlow
54+
});
55+
}
56+
57+
58+
private static void merge(Map<String, Collection<String>> params, Map<String, Object> defaults) {
59+
for(Map.Entry<String, Collection<String>> entry : params.entrySet()) {
60+
String name = entry.getKey();
61+
Collection<String> values = entry.getValue();
62+
defaults.put(name, extractSingleValueIfPossible(values));
63+
}
64+
}
65+
66+
private static Object extractSingleValueIfPossible(Collection<String> values) {
67+
return values.size() == 1 ? values.iterator().next() : ImmutableList.copyOf(values);
68+
}
69+
70+
}

java/ql/test/library-tests/frameworks/ratpack/resources/IntegrationTest.java

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,15 @@ class IntegrationTest {
3131
static class Pojo {
3232
String value;
3333

34+
List<String> values;
35+
3436
String getValue() {
3537
return value;
3638
}
39+
40+
List<String> getValues() {
41+
return values;
42+
}
3743
}
3844

3945
private final ObjectMapper objectMapper = new ObjectMapper();
@@ -112,6 +118,24 @@ void test5(Context ctx) {
112118
});
113119
}
114120

121+
void test6(Context ctx) {
122+
bindQuery(ctx, Pojo.class)
123+
.then(pojo -> {
124+
sink(pojo.getValue()); //$hasTaintFlow
125+
sink(pojo.getValues()); //$hasTaintFlow
126+
});
127+
}
128+
129+
public <T> Promise<T> bindQuery(Context ctx, Class<T> type) {
130+
return bindQuery(ctx, type, Action.noop());
131+
}
132+
133+
public <T> Promise<T> bindQuery(Context ctx, Class<T> type, Action<? super ImmutableMap.Builder<String, Object>> defaults) {
134+
return Promise.sync(() ->
135+
bind(ctx, toObjectNode(ctx.getRequest().getQueryParams(), defaults, name -> false), type)
136+
);
137+
}
138+
115139
private <T> Promise<T> bindJson(Context ctx, Class<T> type) {
116140
return ctx.getRequest().getBody()
117141
.map(data -> {
@@ -215,4 +239,4 @@ public void serializeWithType(Object gen, Object serializers, Object typeSer) th
215239
// empty
216240
}
217241
}
218-
}
242+
}

java/ql/test/library-tests/frameworks/ratpack/resources/Resource.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,5 @@ void test12() {
316316
.then(value -> {
317317
sink(value); // no tainted flow
318318
});
319-
}
320-
321-
}
319+
}
320+
}

0 commit comments

Comments
 (0)