Skip to content

Commit 6562ac3

Browse files
JLLeitschuhJLLeitschuh
committed
Ratpack conversion to new lambda model
Signed-off-by: Jonathan Leitschuh <Jonathan.Leitschuh@gmail.com>
1 parent 4f90f0a commit 6562ac3

File tree

4 files changed

+125
-163
lines changed

4 files changed

+125
-163
lines changed

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,9 @@ private class RatpackModel extends SummaryModelCsv {
7575
row =
7676
["ratpack.util;", "ratpack.func;"] +
7777
[
78-
"MultiValueMap;true;getAll;;;Element of Argument[-1];ReturnValue;value",
79-
"MultiValueMap;true;asMultimap;;;Element of Argument[-1];ReturnValue;value"
78+
"MultiValueMap;true;getAll;;;MapKey of Argument[-1];MapKey of ReturnValue;value",
79+
"MultiValueMap;true;getAll;;;MapValue of Argument[-1];Element of MapValue of ReturnValue;value",
80+
"MultiValueMap;true;asMultimap;;;Element of Argument[-1];Element of ReturnValue;value"
8081
]
8182
}
8283
}

‎java/ql/lib/semmle/code/java/frameworks/ratpack/RatpackExec.qll‎

Lines changed: 45 additions & 148 deletions
Original file line numberDiff line numberDiff line change
@@ -3,174 +3,71 @@ private import semmle.code.java.dataflow.DataFlow
33
private import semmle.code.java.dataflow.FlowSteps
44
private import semmle.code.java.dataflow.ExternalFlow
55

6-
/** A reference type that extends a parameterization the Promise type. */
7-
private class RatpackPromise extends RefType {
8-
RatpackPromise() {
9-
getSourceDeclaration().getASourceSupertype*().hasQualifiedName("ratpack.exec", "Promise")
10-
}
11-
}
12-
13-
/**
6+
/**
147
* Ratpack methods that propagate user-supplied data as tainted.
158
*/
169
private class RatpackExecModel extends SummaryModelCsv {
1710
override predicate row(string row) {
1811
//"namespace;type;overrides;name;signature;ext;inputspec;outputspec;kind",
19-
row = [
20-
"ratpack.exec;Promise;true;map;;;Element of Argument[-1];Parameter[0] of Argument[0];value",
21-
"ratpack.exec;Promise;true;map;;;ReturnValue of Argument[0];Element of ReturnValue;value",
22-
"ratpack.exec;Promise;true;then;;;Element of Argument[-1];Parameter[0] of Argument[0];value"
23-
]
24-
}
25-
}
26-
27-
28-
private class RatpackPromiseValueMethod extends Method, TaintPreservingCallable {
29-
RatpackPromiseValueMethod() { isStatic() and hasName("value") }
30-
31-
override predicate returnsTaintFrom(int arg) { arg = 0 }
32-
}
33-
34-
abstract private class FluentLambdaMethod extends Method {
35-
/**
36-
* Holds if this lambda consumes taint from the quaifier when `lambdaArg`
37-
* for `methodArg` is tainted.
38-
* Eg. `tainted.map(stillTainted -> ..)`
39-
*/
40-
abstract predicate consumesTaint(int methodArg, int lambdaArg);
41-
42-
/**
43-
* Holds if the lambda passed at the given `arg` position produces taint
44-
* that taints the result of this method.
45-
* Eg. `var tainted = CompletableFuture.supplyAsync(() -> taint());`
46-
*/
47-
predicate doesReturnTaint(int arg) { none() }
48-
}
49-
50-
private class RatpackPromiseProviderMethod extends Method, FluentLambdaMethod {
51-
RatpackPromiseProviderMethod() { isStatic() and hasName(["flatten", "sync"]) }
52-
53-
override predicate consumesTaint(int methodArg, int lambdaArg) { none() }
54-
55-
override predicate doesReturnTaint(int arg) { arg = 0 }
56-
}
57-
58-
abstract private class SimpleFluentLambdaMethod extends FluentLambdaMethod {
59-
override predicate consumesTaint(int methodArg, int lambdaArg) {
60-
methodArg = 0 and consumesTaint(lambdaArg)
12+
row =
13+
["ratpack.exec;Promise;true;"] +
14+
[
15+
// `Promise` creation methods
16+
"value;;;Argument[0];Element of ReturnValue;value",
17+
"flatten;;;Element of ReturnValue of Argument[0];Element of ReturnValue;value",
18+
"sync;;;ReturnValue of Argument[0];Element of ReturnValue;value",
19+
// `Promise` value transformation methods
20+
"map;;;Element of Argument[-1];Parameter[0] of Argument[0];value",
21+
"map;;;ReturnValue of Argument[0];Element of ReturnValue;value",
22+
"blockingMap;;;Element of Argument[-1];Parameter[0] of Argument[0];value",
23+
"blockingMap;;;ReturnValue of Argument[0];Element of ReturnValue;value",
24+
"mapError;;;ReturnValue of Argument[0];Element of ReturnValue;value",
25+
// `Promise` termination method
26+
"then;;;Element of Argument[-1];Parameter[0] of Argument[0];value",
27+
// 'next' accesses qualfier the 'Promise' value and also returns the qualifier
28+
"next;;;Element of Argument[-1];Parameter[0] of Argument[0];value",
29+
"next;;;Argument[-1];ReturnValue;value",
30+
// 'cacheIf' accesses qualfier the 'Promise' value and also returns the qualifier
31+
"cacheIf;;;Element of Argument[-1];Parameter[0] of Argument[0];value",
32+
"cacheIf;;;Argument[-1];ReturnValue;value",
33+
// 'route' accesses qualfier the 'Promise' value, and conditionally returns the qualifier or
34+
// the result of the second argument
35+
"route;;;Element of Argument[-1];Parameter[0] of Argument[0];value",
36+
"route;;;Element of Argument[-1];Parameter[0] of Argument[1];value",
37+
"route;;;Argument[-1];ReturnValue;value",
38+
// `flatMap` type methods return their returned `Promise`
39+
"flatMap;;;Element of Argument[-1];Parameter[0] of Argument[0];value",
40+
"flatMap;;;Element of ReturnValue of Argument[0];Element of ReturnValue;value",
41+
"flatMapError;;;Element of ReturnValue of Argument[0];Element of ReturnValue;value",
42+
// `mapIf` methods conditionally map their values, or return themselves
43+
"mapIf;;;Element of Argument[-1];Parameter[0] of Argument[0];value",
44+
"mapIf;;;Element of Argument[-1];Parameter[0] of Argument[1];value",
45+
"mapIf;;;Element of Argument[-1];Parameter[0] of Argument[2];value",
46+
"mapIf;;;ReturnValue of Argument[1];Element of ReturnValue;value",
47+
"mapIf;;;ReturnValue of Argument[2];Element of ReturnValue;value"
48+
]
6149
}
62-
63-
/**
64-
* Holds if this lambda consumes taint from the quaifier when `arg` is tainted.
65-
* Eg. `tainted.map(stillTainted -> ..)`
66-
*/
67-
abstract predicate consumesTaint(int lambdaArg);
6850
}
6951

70-
private class RatpackPromiseMapMethod extends SimpleFluentLambdaMethod {
71-
RatpackPromiseMapMethod() {
72-
getDeclaringType() instanceof RatpackPromise and
73-
hasName(["blockingMap"]) // "flatMap" & "apply" cause false positives. Wait for fluent lambda support.
52+
/** A reference type that extends a parameterization the Promise type. */
53+
private class RatpackPromise extends RefType {
54+
RatpackPromise() {
55+
getSourceDeclaration().getASourceSupertype*().hasQualifiedName("ratpack.exec", "Promise")
7456
}
75-
76-
override predicate consumesTaint(int lambdaArg) { lambdaArg = 0 }
77-
78-
override predicate doesReturnTaint(int arg) { arg = 0 }
7957
}
8058

8159
/**
82-
* Represents the `mapIf` method.
83-
*
84-
* `<O> Promise<O> mapIf(Predicate<T> predicate, Function<T, O> onTrue, Function<T, O> onFalse)`
60+
* Ratpack `Promise` method that will return `this`.
8561
*/
86-
private class RatpackPromiseMapIfMethod extends FluentLambdaMethod {
87-
RatpackPromiseMapIfMethod() {
88-
getDeclaringType() instanceof RatpackPromise and
89-
hasName(["mapIf"]) and // "flatMapIf" causes false positives. Wait for fluent lambda support.
90-
getNumberOfParameters() = 3
91-
}
92-
93-
override predicate consumesTaint(int methodArg, int lambdaArg) {
94-
methodArg = [1, 2, 3] and lambdaArg = 0
95-
}
96-
97-
override predicate doesReturnTaint(int arg) { arg = [1, 2] }
98-
}
99-
100-
private class RatpackPromiseMapErrorMethod extends FluentLambdaMethod {
101-
RatpackPromiseMapErrorMethod() {
102-
getDeclaringType() instanceof RatpackPromise and
103-
hasName(["mapError"]) // "flatMapError" causes false positives. Wait for fluent lambda support.
104-
}
105-
106-
override predicate consumesTaint(int methodArg, int lambdaArg) { none() }
107-
108-
override predicate doesReturnTaint(int arg) { arg = getNumberOfParameters() - 1 }
109-
}
110-
111-
// private class RatpackPromiseThenMethod extends SimpleFluentLambdaMethod {
112-
// RatpackPromiseThenMethod() {
113-
// getDeclaringType() instanceof RatpackPromise and
114-
// hasName("then")
115-
// }
116-
117-
// override predicate consumesTaint(int lambdaArg) { lambdaArg = 0 }
118-
// }
119-
120-
private class RatpackPromiseFluentMethod extends FluentMethod, FluentLambdaMethod {
62+
private class RatpackPromiseFluentMethod extends FluentMethod {
12163
RatpackPromiseFluentMethod() {
12264
getDeclaringType() instanceof RatpackPromise and
12365
not isStatic() and
66+
// It's generally safe to assume that if the return type exactly matches the declaring type, `this` will be returned.
12467
exists(ParameterizedType t |
12568
t instanceof RatpackPromise and
12669
t = getDeclaringType() and
12770
t = getReturnType()
12871
)
12972
}
130-
131-
override predicate consumesTaint(int methodArg, int lambdaArg) {
132-
hasName(["next"]) and methodArg = 0 and lambdaArg = 0
133-
or
134-
hasName(["cacheIf"]) and methodArg = 0 and lambdaArg = 0
135-
or
136-
hasName(["route"]) and methodArg = [0, 1] and lambdaArg = 0
137-
}
138-
139-
override predicate doesReturnTaint(int arg) { none() } // "flatMapIf" causes false positives. Wait for fluent lambda support.
140-
}
141-
142-
/**
143-
* Holds if the method access qualifier `node1` has dataflow to the functional expression parameter `node2`.
144-
*/
145-
private predicate stepIntoLambda(DataFlow::Node node1, DataFlow::Node node2) {
146-
exists(MethodAccess ma, FluentLambdaMethod flm, int methodArg, int lambdaArg |
147-
flm.consumesTaint(methodArg, lambdaArg)
148-
|
149-
ma.getMethod() = flm and
150-
node1.asExpr() = ma.getQualifier() and
151-
ma.getArgument(methodArg).(FunctionalExpr).asMethod().getParameter(lambdaArg) =
152-
node2.asParameter()
153-
)
154-
}
155-
156-
/**
157-
* Holds if the return statement result of the functional expression `node1` has dataflow to the
158-
* method access result `node2`.
159-
*/
160-
private predicate stepOutOfLambda(DataFlow::Node node1, DataFlow::Node node2) {
161-
exists(FluentLambdaMethod flm, MethodAccess ma, FunctionalExpr fe, int arg |
162-
flm.doesReturnTaint(arg)
163-
|
164-
fe.asMethod().getBody().getAStmt().(ReturnStmt).getResult() = node1.asExpr() and
165-
ma.getMethod() = flm and
166-
node2.asExpr() = ma and
167-
ma.getArgument(arg) = fe
168-
)
169-
}
170-
171-
private class RatpackPromiseTaintPreservingStep extends AdditionalTaintStep {
172-
override predicate step(DataFlow::Node node1, DataFlow::Node node2) {
173-
stepIntoLambda(node1, node2) or
174-
stepOutOfLambda(node1, node2)
175-
}
17673
}

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

Lines changed: 69 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -53,13 +53,16 @@ void test2(Context ctx, OutputStream os) {
5353
}
5454

5555
void test3(Context ctx) {
56-
sink(ctx.getRequest().getBody().map(TypedData::getText)); //$hasTaintFlow
57-
Promise<String> mapResult = ctx.getRequest().getBody().map(b -> {
56+
ctx.getRequest().getBody().map(TypedData::getText).then(s -> {
57+
sink(s); //$hasTaintFlow
58+
});
59+
ctx.getRequest().getBody().map(b -> {
5860
sink(b); //$hasTaintFlow
5961
sink(b.getText()); //$hasTaintFlow
6062
return b.getText();
63+
}).then(t -> {
64+
sink(t); //$hasTaintFlow
6165
});
62-
sink(mapResult); //$hasTaintFlow
6366
ctx.getRequest().getBody().map(TypedData::getText).then(this::sink); //$hasTaintFlow
6467
ctx
6568
.getRequest()
@@ -72,7 +75,9 @@ void test3(Context ctx) {
7275
void test4() {
7376
String tainted = taint();
7477
Promise.value(tainted);
75-
sink(Promise.value(tainted)); //$hasTaintFlow
78+
Promise
79+
.value(tainted)
80+
.then(this::sink); //$hasTaintFlow
7681
Promise
7782
.value(tainted)
7883
.map(a -> a)
@@ -188,15 +193,14 @@ void test8() {
188193
.then(value -> {
189194
sink(value); //$hasTaintFlow
190195
});
191-
// Waiting for support for lambda data flow
192-
// Promise
193-
// .value("potato")
194-
// .flatMapError(RuntimeException.class, exception -> {
195-
// return Promise.value(taint());
196-
// })
197-
// .then(value -> {
198-
// sink(value); //$hasTaintFlow
199-
// });
196+
Promise
197+
.value("potato")
198+
.flatMapError(RuntimeException.class, exception -> {
199+
return Promise.value(taint());
200+
})
201+
.then(value -> {
202+
sink(value); //$hasTaintFlow
203+
});
200204
}
201205

202206
void test9() {
@@ -213,6 +217,12 @@ void test9() {
213217
.then(value -> {
214218
sink(value); // no taints flow
215219
});
220+
Promise
221+
.value(tainted)
222+
.flatMap(v -> Promise.value(v))
223+
.then(value -> {
224+
sink(value); //$hasTaintFlow
225+
});
216226
}
217227

218228
public static String identity(String input) {
@@ -234,5 +244,51 @@ void test10() {
234244
sink(value); // no taints flow
235245
});
236246
}
247+
248+
void test11() {
249+
String tainted = taint();
250+
Promise
251+
.sync(() -> tainted)
252+
.mapIf(v -> {
253+
sink(v); //$hasTaintFlow
254+
return true;
255+
}, v -> {
256+
sink(v); //$hasTaintFlow
257+
return v;
258+
})
259+
.then(value -> {
260+
sink(value); //$hasTaintFlow
261+
});
262+
Promise
263+
.sync(() -> tainted)
264+
.mapIf(v -> {
265+
sink(v); //$hasTaintFlow
266+
return true;
267+
}, vTrue -> {
268+
sink(vTrue); //$hasTaintFlow
269+
return vTrue;
270+
}, vFalse -> {
271+
sink(vFalse); //$hasTaintFlow
272+
return vFalse;
273+
})
274+
.then(value -> {
275+
sink(value); //$hasTaintFlow
276+
});
277+
Promise
278+
.sync(() -> tainted)
279+
.mapIf(v -> {
280+
sink(v); //$hasTaintFlow
281+
return true;
282+
}, vTrue -> {
283+
sink(vTrue); //$hasTaintFlow
284+
return "potato";
285+
}, vFalse -> {
286+
sink(vFalse); //$hasTaintFlow
287+
return "potato";
288+
})
289+
.then(value -> {
290+
sink(value); // no tainted flow
291+
});
292+
}
237293

238294
}

‎java/ql/test/stubs/ratpack-1.9.x/ratpack/exec/Promise.java‎

Lines changed: 8 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)