Skip to content

Commit 42a7208

Browse files
committed
JS: Migrate ExceptionXss
1 parent d9a43db commit 42a7208

File tree

2 files changed

+52
-15
lines changed

2 files changed

+52
-15
lines changed

javascript/ql/lib/semmle/javascript/security/dataflow/ExceptionXssCustomizations.qll

Lines changed: 40 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,15 +14,48 @@ import javascript
1414
module ExceptionXss {
1515
private import Xss::Shared as Shared
1616

17+
private newtype TFlowState =
18+
TThrown() or
19+
TNotYetThrown()
20+
21+
/** A flow state to associate with a tracked value. */
22+
class FlowState extends TFlowState {
23+
/** Gets a string representation fo this flow state */
24+
string toString() {
25+
this = TThrown() and result = "thrown"
26+
or
27+
this = TNotYetThrown() and result = "not-yet-thrown"
28+
}
29+
30+
deprecated DataFlow::FlowLabel toFlowLabel() {
31+
this = TThrown() and result.isTaint()
32+
or
33+
this = TNotYetThrown() and result instanceof NotYetThrown
34+
}
35+
}
36+
37+
/** Predicates for working with flow states. */
38+
module FlowState {
39+
deprecated FlowState fromFlowLabel(DataFlow::FlowLabel label) { result.toFlowLabel() = label }
40+
41+
/** A tainted value originating from a thrown and caught exception. */
42+
FlowState thrown() { result = TThrown() }
43+
44+
/** A value that has not yet been thrown. */
45+
FlowState notYetThrown() { result = TNotYetThrown() }
46+
}
47+
1748
/** A data flow source for XSS caused by interpreting exception or error text as HTML. */
1849
abstract class Source extends DataFlow::Node {
1950
/**
20-
* Gets a flow label to associate with this source.
51+
* Gets a flow state to associate with this source.
2152
*
2253
* For sources that should pass through a `throw/catch` before reaching the sink, use the
23-
* `NotYetThrown` labe. Otherwise use `taint` (the default).
54+
* `FlowState::notYetThrown()` state. Otherwise use `FlowState::thrown()` (the default).
2455
*/
25-
DataFlow::FlowLabel getAFlowLabel() { result.isTaint() }
56+
FlowState getAFlowState() { result = FlowState::thrown() }
57+
58+
deprecated DataFlow::FlowLabel getAFlowLabel() { result = this.getAFlowState().toFlowLabel() }
2659

2760
/**
2861
* Gets a human-readable description of what type of error this refers to.
@@ -33,17 +66,19 @@ module ExceptionXss {
3366
}
3467

3568
/**
69+
* DEPRECATED. Use `FlowState` instead.
70+
*
3671
* A FlowLabel representing tainted data that has not been thrown in an exception.
3772
* In the js/xss-through-exception query data-flow can only reach a sink after
3873
* the data has been thrown as an exception, and data that has not been thrown
3974
* as an exception therefore has this flow label, and only this flow label, associated with it.
4075
*/
41-
abstract class NotYetThrown extends DataFlow::FlowLabel {
76+
abstract deprecated class NotYetThrown extends DataFlow::FlowLabel {
4277
NotYetThrown() { this = "NotYetThrown" }
4378
}
4479

4580
private class XssSourceAsSource extends Source instanceof Shared::Source {
46-
override DataFlow::FlowLabel getAFlowLabel() { result instanceof NotYetThrown }
81+
override FlowState getAFlowState() { result instanceof TNotYetThrown }
4782

4883
override string getDescription() { result = "Exception text" }
4984
}

javascript/ql/lib/semmle/javascript/security/dataflow/ExceptionXssQuery.qll

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import javascript
88
import DomBasedXssCustomizations::DomBasedXss as DomBasedXssCustom
99
import ReflectedXssCustomizations::ReflectedXss as ReflectedXssCustom
1010
import ExceptionXssCustomizations::ExceptionXss
11+
private import ExceptionXssCustomizations::ExceptionXss as ExceptionXss
1112
private import semmle.javascript.dataflow.InferredTypes
1213
import Xss::Shared as XssShared
1314

@@ -71,7 +72,7 @@ predicate canThrowSensitiveInformation(DataFlow::Node node) {
7172
}
7273

7374
// Materialize flow labels
74-
private class ConcreteNotYetThrown extends NotYetThrown {
75+
deprecated private class ConcreteNotYetThrown extends NotYetThrown {
7576
ConcreteNotYetThrown() { this = this }
7677
}
7778

@@ -130,25 +131,25 @@ private DataFlow::Node getExceptionTarget(DataFlow::Node pred) {
130131
* an exception.
131132
*/
132133
module ExceptionXssConfig implements DataFlow::StateConfigSig {
133-
class FlowState = DataFlow::FlowLabel;
134+
class FlowState = ExceptionXss::FlowState;
134135

135-
predicate isSource(DataFlow::Node source, DataFlow::FlowLabel label) {
136-
source.(Source).getAFlowLabel() = label
136+
predicate isSource(DataFlow::Node source, FlowState state) {
137+
source.(Source).getAFlowState() = state
137138
}
138139

139-
predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel label) {
140-
sink instanceof XssShared::Sink and not label instanceof NotYetThrown
140+
predicate isSink(DataFlow::Node sink, FlowState state) {
141+
sink instanceof XssShared::Sink and not state = FlowState::notYetThrown()
141142
}
142143

143144
predicate isBarrier(DataFlow::Node node) {
144145
node instanceof XssShared::Sanitizer or node = XssShared::BarrierGuard::getABarrierNode()
145146
}
146147

147148
predicate isAdditionalFlowStep(
148-
DataFlow::Node pred, DataFlow::FlowLabel inlbl, DataFlow::Node succ, DataFlow::FlowLabel outlbl
149+
DataFlow::Node pred, FlowState inlbl, DataFlow::Node succ, FlowState outlbl
149150
) {
150-
inlbl instanceof NotYetThrown and
151-
(outlbl.isTaint() or outlbl instanceof NotYetThrown) and
151+
inlbl = FlowState::notYetThrown() and
152+
outlbl = [FlowState::thrown(), FlowState::notYetThrown()] and
152153
canThrowSensitiveInformation(pred) and
153154
succ = getExceptionTarget(pred)
154155
}
@@ -178,7 +179,8 @@ deprecated class Configuration extends TaintTracking::Configuration {
178179
override predicate isAdditionalFlowStep(
179180
DataFlow::Node pred, DataFlow::Node succ, DataFlow::FlowLabel inlbl, DataFlow::FlowLabel outlbl
180181
) {
181-
ExceptionXssConfig::isAdditionalFlowStep(pred, inlbl, succ, outlbl)
182+
ExceptionXssConfig::isAdditionalFlowStep(pred, FlowState::fromFlowLabel(inlbl), succ,
183+
FlowState::fromFlowLabel(outlbl))
182184
or
183185
// All the usual taint-flow steps apply on data-flow before it has been thrown in an exception.
184186
// Note: this step is not needed in StateConfigSig module since flow states inherit taint steps.

0 commit comments

Comments
 (0)