Skip to content

Commit 4f90f0a

Browse files
committed
Begin refactoring Ratpack to use functional taint tracking
Signed-off-by: Jonathan Leitschuh <[email protected]>
1 parent 6497a61 commit 4f90f0a

File tree

5 files changed

+38
-12
lines changed

5 files changed

+38
-12
lines changed

java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,8 @@ private module Frameworks {
9595
private import semmle.code.java.frameworks.Optional
9696
private import semmle.code.java.frameworks.Stream
9797
private import semmle.code.java.frameworks.Strings
98+
private import semmle.code.java.frameworks.ratpack.Ratpack
99+
private import semmle.code.java.frameworks.ratpack.RatpackExec
98100
private import semmle.code.java.frameworks.spring.SpringCache
99101
private import semmle.code.java.frameworks.spring.SpringHttp
100102
private import semmle.code.java.frameworks.spring.SpringUtil

java/ql/src/semmle/code/java/frameworks/ratpack/Ratpack.qll renamed to java/ql/lib/semmle/code/java/frameworks/ratpack/Ratpack.qll

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,8 @@ private class RatpackHttpSource extends SourceModelCsv {
2929
row =
3030
["ratpack.handling;", "ratpack.core.handling;"] + "Context;true;parse;" +
3131
[
32-
"(java.lang.Class);", "(com.google.common.reflect.TypeToken);", "(java.lang.Class,java.lang.Object);",
32+
"(java.lang.Class);", "(com.google.common.reflect.TypeToken);",
33+
"(java.lang.Class,java.lang.Object);",
3334
"(com.google.common.reflect.TypeToken,java.lang.Object);", "(ratpack.core.parse.Parse);",
3435
"(ratpack.parse.Parse);"
3536
] + ";ReturnValue;remote"
@@ -74,8 +75,8 @@ private class RatpackModel extends SummaryModelCsv {
7475
row =
7576
["ratpack.util;", "ratpack.func;"] +
7677
[
77-
"MultiValueMap;true;getAll;;;Argument[-1];ReturnValue;taint",
78-
"MultiValueMap;true;asMultimap;;;Argument[-1];ReturnValue;taint"
78+
"MultiValueMap;true;getAll;;;Element of Argument[-1];ReturnValue;value",
79+
"MultiValueMap;true;asMultimap;;;Element of Argument[-1];ReturnValue;value"
7980
]
8081
}
8182
}

java/ql/src/semmle/code/java/frameworks/ratpack/RatpackExec.qll renamed to java/ql/lib/semmle/code/java/frameworks/ratpack/RatpackExec.qll

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import java
22
private import semmle.code.java.dataflow.DataFlow
33
private import semmle.code.java.dataflow.FlowSteps
4+
private import semmle.code.java.dataflow.ExternalFlow
45

56
/** A reference type that extends a parameterization the Promise type. */
67
private class RatpackPromise extends RefType {
@@ -9,6 +10,21 @@ private class RatpackPromise extends RefType {
910
}
1011
}
1112

13+
/**
14+
* Ratpack methods that propagate user-supplied data as tainted.
15+
*/
16+
private class RatpackExecModel extends SummaryModelCsv {
17+
override predicate row(string row) {
18+
//"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+
1228
private class RatpackPromiseValueMethod extends Method, TaintPreservingCallable {
1329
RatpackPromiseValueMethod() { isStatic() and hasName("value") }
1430

@@ -54,7 +70,7 @@ abstract private class SimpleFluentLambdaMethod extends FluentLambdaMethod {
5470
private class RatpackPromiseMapMethod extends SimpleFluentLambdaMethod {
5571
RatpackPromiseMapMethod() {
5672
getDeclaringType() instanceof RatpackPromise and
57-
hasName(["map", "blockingMap"]) // "flatMap" & "apply" cause false positives. Wait for fluent lambda support.
73+
hasName(["blockingMap"]) // "flatMap" & "apply" cause false positives. Wait for fluent lambda support.
5874
}
5975

6076
override predicate consumesTaint(int lambdaArg) { lambdaArg = 0 }
@@ -92,14 +108,14 @@ private class RatpackPromiseMapErrorMethod extends FluentLambdaMethod {
92108
override predicate doesReturnTaint(int arg) { arg = getNumberOfParameters() - 1 }
93109
}
94110

95-
private class RatpackPromiseThenMethod extends SimpleFluentLambdaMethod {
96-
RatpackPromiseThenMethod() {
97-
getDeclaringType() instanceof RatpackPromise and
98-
hasName("then")
99-
}
111+
// private class RatpackPromiseThenMethod extends SimpleFluentLambdaMethod {
112+
// RatpackPromiseThenMethod() {
113+
// getDeclaringType() instanceof RatpackPromise and
114+
// hasName("then")
115+
// }
100116

101-
override predicate consumesTaint(int lambdaArg) { lambdaArg = 0 }
102-
}
117+
// override predicate consumesTaint(int lambdaArg) { lambdaArg = 0 }
118+
// }
103119

104120
private class RatpackPromiseFluentMethod extends FluentMethod, FluentLambdaMethod {
105121
RatpackPromiseFluentMethod() {
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/ratpack-1.9.x:${testdir}/../../../stubs/jackson-databind-2.10:${testdir}/../../../stubs/guava-30.0:${testdir}/../../../stubs/guava-30.0:${testdir}/../../../stubs/netty-4.1.x
1+
//semmle-extractor-options: --javac-args -cp ${testdir}/../../../stubs/ratpack-1.9.x:${testdir}/../../../stubs/jackson-databind-2.12:${testdir}/../../../stubs/guava-30.0:${testdir}/../../../stubs/guava-30.0:${testdir}/../../../stubs/netty-4.1.x

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ void test1(Context ctx) {
3636

3737
void test2(Context ctx, OutputStream os) {
3838
ctx.getRequest().getBody().then(td -> {
39+
sink(td); //$hasTaintFlow
3940
sink(td.getText()); //$hasTaintFlow
4041
sink(td.getBuffer()); //$hasTaintFlow
4142
sink(td.getBytes()); //$hasTaintFlow
@@ -53,6 +54,12 @@ void test2(Context ctx, OutputStream os) {
5354

5455
void test3(Context ctx) {
5556
sink(ctx.getRequest().getBody().map(TypedData::getText)); //$hasTaintFlow
57+
Promise<String> mapResult = ctx.getRequest().getBody().map(b -> {
58+
sink(b); //$hasTaintFlow
59+
sink(b.getText()); //$hasTaintFlow
60+
return b.getText();
61+
});
62+
sink(mapResult); //$hasTaintFlow
5663
ctx.getRequest().getBody().map(TypedData::getText).then(this::sink); //$hasTaintFlow
5764
ctx
5865
.getRequest()

0 commit comments

Comments
 (0)