Skip to content

Commit f8922f1

Browse files
committed
Java: XSS with abstract class (some duplication)
I haven't figured out how to avoid the redundancy between `getASelected{Source,Sink}Location` in the module and the class. Maybe we need a strong notion of primary and secondary data-flow configurations.
1 parent 7c7d639 commit f8922f1

File tree

4 files changed

+49
-1
lines changed

4 files changed

+49
-1
lines changed

java/ql/lib/semmle/code/java/security/XSS.qll

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,26 @@ private module XssVulnerableWriterSourceToWritingMethodFlowConfig implements Dat
7171
sink.asExpr() = ma.getQualifier() and ma.getMethod() instanceof WritingMethod
7272
)
7373
}
74+
75+
predicate observeDiffInformedIncrementalMode() {
76+
// Since this configuration is for finding sinks to be used in a main
77+
// data-flow configuration, this configuration should only restrict the
78+
// sinks to be found if there are no main-configuration sources in the
79+
// diff range. That's because if there is such a source, we need to
80+
// report query results for it even with sinks outside the diff range.
81+
exists(DataFlow::DiffInformedQuery q | not q.hasSourceInDiffRange())
82+
}
83+
84+
// The sources are not exposed outside this file module, so we know the
85+
// query will not select them.
86+
Location getASelectedSourceLocation(DataFlow::Node source) { none() }
87+
88+
Location getASelectedSinkLocation(DataFlow::Node sink) {
89+
// This code mirrors `DefaultXssSink()`.
90+
exists(MethodCall ma | result = ma.getAnArgument().getLocation() |
91+
sink.asExpr() = ma.getQualifier() and ma.getMethod() instanceof WritingMethod
92+
)
93+
}
7494
}
7595

7696
private module XssVulnerableWriterSourceToWritingMethodFlow =

java/ql/lib/semmle/code/java/security/XssQuery.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ module XssConfig implements DataFlow::ConfigSig {
2121
any(XssAdditionalTaintStep s).step(node1, node2)
2222
}
2323

24-
predicate observeDiffInformedIncrementalMode() { any() }
24+
predicate observeDiffInformedIncrementalMode() { exists(DataFlow::DiffInformedQuery q) }
2525
}
2626

2727
/** Tracks flow from remote sources to cross site scripting vulnerabilities. */

java/ql/src/Security/CWE/CWE-079/XSS.ql

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,16 @@ import java
1515
import semmle.code.java.security.XssQuery
1616
import XssFlow::PathGraph
1717

18+
class IsDiffInformed extends DataFlow::DiffInformedQuery {
19+
// This predicate is overridden to be more precise than the default
20+
// implementation in order to support secondary secondary data-flow
21+
// configurations that find sinks.
22+
override Location getASelectedSourceLocation(DataFlow::Node source) {
23+
XssConfig::isSource(source) and
24+
result = source.getLocation()
25+
}
26+
}
27+
1828
from XssFlow::PathNode source, XssFlow::PathNode sink
1929
where XssFlow::flowPath(source, sink)
2030
select sink.getNode(), source, sink, "Cross-site scripting vulnerability due to a $@.",

shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
1818
private import DataFlowImplCommon::MakeImplCommon<Location, Lang>
1919
private import DataFlowImplCommonPublic
2020
private import Aliases
21+
private import codeql.util.AlertFiltering
22+
23+
private module AlertFiltering = AlertFilteringImpl<Location>;
2124

2225
/**
2326
* An input configuration for data flow using flow state. This signature equals
@@ -180,6 +183,21 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
180183
DiffInformedQueryImpl() { any() }
181184

182185
string toString() { result = "DataFlow::DiffInformedQuery" }
186+
187+
// TODO: how to wire this up? It overlaps with the data-flow configuration signature!
188+
Location getASelectedSourceLocation(Node source) { result = source.getLocation() }
189+
190+
Location getASelectedSinkLocation(Node sink) { result = sink.getLocation() }
191+
192+
pragma[nomagic]
193+
predicate hasSourceInDiffRange() {
194+
AlertFiltering::filterByLocation(this.getASelectedSourceLocation(_))
195+
}
196+
197+
pragma[nomagic]
198+
predicate hasSinkInDiffRange() {
199+
AlertFiltering::filterByLocation(this.getASelectedSinkLocation(_))
200+
}
183201
}
184202

185203
/**

0 commit comments

Comments
 (0)