Skip to content

Commit 36ff54b

Browse files
committed
Convert jump step into local step
Note that this has FNs in the test cases where the source is used locally in the nested classes' methods
1 parent cc5a404 commit 36ff54b

File tree

1 file changed

+25
-9
lines changed

1 file changed

+25
-9
lines changed

java/ql/lib/semmle/code/java/frameworks/InputStream.qll

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,27 +3,36 @@
33
import java
44
private import semmle.code.java.dataflow.DataFlow
55
private import semmle.code.java.dataflow.FlowSteps
6+
private import semmle.code.java.dataflow.SSA
7+
private import semmle.code.java.dataflow.TaintTracking
68

79
/**
8-
* A taint step from an update of the `bytes[]` parameter in an override of the `InputStream.read` method
10+
* A local taint step from the definition of a captured variable, the capturer of which
11+
* updates the `bytes[]` parameter in an override of the `InputStream.read` method,
912
* to a class instance expression of the type extending `InputStream`.
1013
*
11-
* This models how a subtype of `InputStream` could be tainted by the definition of its methods, which will
12-
* normally only happen in anonymous classes.
14+
* This models how a subtype of `InputStream` could be tainted by capturing tainted variables in
15+
* the definition of its methods.
1316
*/
14-
private class InputStreamWrapperAnonymousStep extends AdditionalTaintStep {
17+
private class InputStreamWrapperCapturedLocalStep extends AdditionalTaintStep {
1518
override predicate step(DataFlow::Node n1, DataFlow::Node n2) {
16-
exists(Method m, NestedClass wrapper |
17-
m.hasName("read") and
19+
exists(InputStreamRead m, NestedClass wrapper, SsaVariable captured, SsaImplicitInit capturer |
20+
wrapper.getASourceSupertype+() instanceof TypeInputStream and
1821
m.getDeclaringType() = wrapper and
19-
wrapper.getASourceSupertype+() instanceof TypeInputStream
20-
|
21-
n1.(DataFlow::PostUpdateNode).getPreUpdateNode().asExpr() = m.getParameter(0).getAnAccess() and
22+
capturer.captures(captured) and
23+
TaintTracking::localTaint(DataFlow::exprNode(capturer.getAFirstUse()),
24+
any(DataFlow::PostUpdateNode pun |
25+
pun.getPreUpdateNode().asExpr() = m.getParameter(0).getAnAccess()
26+
)) and
2227
n2.asExpr()
2328
.(ClassInstanceExpr)
2429
.getConstructedType()
2530
.getASourceSupertype*()
2631
.getSourceDeclaration() = wrapper
32+
|
33+
n1.asExpr() = captured.(SsaExplicitUpdate).getDefiningExpr().(VariableAssign).getSource()
34+
or
35+
captured.(SsaImplicitInit).isParameterDefinition(n1.asParameter())
2736
)
2837
}
2938
}
@@ -49,3 +58,10 @@ private class InputStreamWrapperConstructorStep extends AdditionalTaintStep {
4958
)
5059
}
5160
}
61+
62+
private class InputStreamRead extends Method {
63+
InputStreamRead() {
64+
this.hasName("read") and
65+
this.getDeclaringType().getASourceSupertype*() instanceof TypeInputStream
66+
}
67+
}

0 commit comments

Comments
 (0)