Skip to content

Commit 3edd65f

Browse files
erik-krogherik-krogh
committed
changed the exceptional taint-steps to step through each call-site
1 parent e95ccee commit 3edd65f

File tree

2 files changed

+50
-18
lines changed

2 files changed

+50
-18
lines changed

‎javascript/ql/src/semmle/javascript/security/dataflow/ExceptionXss.qll‎

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,14 @@ module ExceptionXss {
1414
import Xss::StoredXss as StoredXss
1515
import Xss as XSS
1616

17+
DataFlow::ExceptionalInvocationReturnNode getCallerExceptionalReturn(DataFlow::FunctionNode func) {
18+
exists(DataFlow::InvokeNode call |
19+
not call.isImprecise() and
20+
func.getFunction() = call.(DataFlow::InvokeNode).getACallee() and
21+
result = call.getExceptionalReturn()
22+
)
23+
}
24+
1725
DataFlow::Node getExceptionalSuccssor(DataFlow::Node pred) {
1826
exists(DataFlow::FunctionNode func |
1927
pred.getContainer() = func.getFunction() and
@@ -22,14 +30,12 @@ module ExceptionXss {
2230
result.(DataFlow::ParameterNode).getParameter() = getEnclosingTryStmt(pred
2331
.asExpr()
2432
.getEnclosingStmt()).getACatchClause().getAParameter()
25-
else result = getExceptionalSuccssor(func.getExceptionalReturn())
26-
or
27-
pred = func.getExceptionalReturn() and
28-
exists(DataFlow::InvokeNode call |
29-
not call.isImprecise() and
30-
func.getFunction() = call.(DataFlow::InvokeNode).getACallee() and
31-
result = getExceptionalSuccssor(call)
32-
)
33+
else result = getCallerExceptionalReturn(func)
34+
)
35+
or
36+
exists(DataFlow::InvokeNode call |
37+
pred = call.getExceptionalReturn() and
38+
result = getExceptionalSuccssor(call)
3339
)
3440
}
3541

@@ -53,7 +59,9 @@ module ExceptionXss {
5359
}
5460

5561
/**
56-
* A taint-tracking configuration for reasoning about XSS with possible exceptional flow.
62+
* A taint-tracking configuration for reasoning about XSS with possible exceptional flow.
63+
* Flow labels are used to ensure that we only report taint-flow that has been thrown in
64+
* an exception.
5765
*/
5866
class Configuration extends TaintTracking::Configuration {
5967
Configuration() { this = "ExceptionXss"}
@@ -63,21 +71,23 @@ module ExceptionXss {
6371
}
6472

6573
override predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel label) {
66-
sink instanceof XSS::Shared::Sink and label.isDataOrTaint()
67-
}
74+
sink instanceof XSS::Shared::Sink and not label instanceof NotYetThrown
75+
}
6876

6977
override predicate isSanitizer(DataFlow::Node node) {
7078
super.isSanitizer(node) or
7179
node instanceof XSS::Shared::Sanitizer
7280
}
7381

7482
override predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ, DataFlow::FlowLabel inlbl, DataFlow::FlowLabel outlbl) {
83+
inlbl instanceof NotYetThrown and (outlbl.isTaint() or outlbl instanceof NotYetThrown) and
84+
succ = getExceptionalSuccssor(pred) and
85+
(canThrowSensitiveInformation(pred) or pred = any(DataFlow::InvokeNode c).getExceptionalReturn())
86+
or
87+
// All the usual taint-flow steps applies on data-flow before it has been thrown in an exception.
7588
this.isAdditionalFlowStep(pred, succ) and inlbl instanceof NotYetThrown and outlbl instanceof NotYetThrown
7689
or
77-
inlbl instanceof NotYetThrown and outlbl.isTaint() and
78-
succ = getExceptionalSuccssor(pred) and
79-
canThrowSensitiveInformation(pred)
80-
or
90+
// We taint an object deep if it happens before an exception has been thrown.
8191
inlbl instanceof NotYetThrown and outlbl instanceof NotYetThrown and
8292
exists(DataFlow::PropWrite write | write.getRhs() = pred and write.getBase() = succ)
8393
}

‎javascript/ql/test/query-tests/Security/CWE-079/ExceptionXss.expected‎

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ nodes
88
| exception-xss.js:10:10:10:10 | e |
99
| exception-xss.js:11:18:11:18 | e |
1010
| exception-xss.js:11:18:11:18 | e |
11+
| exception-xss.js:15:3:15:12 | exceptional return of inner(foo) |
12+
| exception-xss.js:15:3:15:12 | exceptional return of inner(foo) |
1113
| exception-xss.js:15:9:15:11 | foo |
1214
| exception-xss.js:16:10:16:10 | e |
1315
| exception-xss.js:17:18:17:18 | e |
@@ -28,16 +30,23 @@ nodes
2830
| exception-xss.js:35:18:35:18 | e |
2931
| exception-xss.js:35:18:35:18 | e |
3032
| exception-xss.js:38:16:38:16 | x |
33+
| exception-xss.js:39:3:39:10 | exceptional return of deep2(x) |
3134
| exception-xss.js:39:9:39:9 | x |
3235
| exception-xss.js:41:17:41:17 | x |
36+
| exception-xss.js:42:3:42:10 | exceptional return of inner(x) |
3337
| exception-xss.js:42:9:42:9 | x |
38+
| exception-xss.js:46:3:46:19 | exceptional return of deep("bar" + foo) |
39+
| exception-xss.js:46:3:46:19 | exceptional return of deep("bar" + foo) |
3440
| exception-xss.js:46:8:46:18 | "bar" + foo |
3541
| exception-xss.js:46:16:46:18 | foo |
3642
| exception-xss.js:47:10:47:10 | e |
3743
| exception-xss.js:48:18:48:18 | e |
3844
| exception-xss.js:48:18:48:18 | e |
3945
| exception-xss.js:74:28:74:28 | x |
46+
| exception-xss.js:75:4:75:11 | exceptional return of inner(x) |
4047
| exception-xss.js:75:10:75:10 | x |
48+
| exception-xss.js:81:3:81:19 | exceptional return of myWeirdInner(foo) |
49+
| exception-xss.js:81:3:81:19 | exceptional return of myWeirdInner(foo) |
4150
| exception-xss.js:81:16:81:18 | foo |
4251
| exception-xss.js:82:10:82:10 | e |
4352
| exception-xss.js:83:18:83:18 | e |
@@ -71,12 +80,15 @@ edges
7180
| exception-xss.js:2:15:2:31 | document.location | exception-xss.js:2:9:2:31 | foo |
7281
| exception-xss.js:2:15:2:31 | document.location | exception-xss.js:2:9:2:31 | foo |
7382
| exception-xss.js:4:20:4:20 | x | exception-xss.js:5:14:5:14 | x |
74-
| exception-xss.js:5:14:5:14 | x | exception-xss.js:16:10:16:10 | e |
75-
| exception-xss.js:5:14:5:14 | x | exception-xss.js:47:10:47:10 | e |
76-
| exception-xss.js:5:14:5:14 | x | exception-xss.js:82:10:82:10 | e |
83+
| exception-xss.js:5:14:5:14 | x | exception-xss.js:15:3:15:12 | exceptional return of inner(foo) |
84+
| exception-xss.js:5:14:5:14 | x | exception-xss.js:15:3:15:12 | exceptional return of inner(foo) |
85+
| exception-xss.js:5:14:5:14 | x | exception-xss.js:42:3:42:10 | exceptional return of inner(x) |
86+
| exception-xss.js:5:14:5:14 | x | exception-xss.js:75:4:75:11 | exceptional return of inner(x) |
7787
| exception-xss.js:9:11:9:13 | foo | exception-xss.js:10:10:10:10 | e |
7888
| exception-xss.js:10:10:10:10 | e | exception-xss.js:11:18:11:18 | e |
7989
| exception-xss.js:10:10:10:10 | e | exception-xss.js:11:18:11:18 | e |
90+
| exception-xss.js:15:3:15:12 | exceptional return of inner(foo) | exception-xss.js:16:10:16:10 | e |
91+
| exception-xss.js:15:3:15:12 | exceptional return of inner(foo) | exception-xss.js:16:10:16:10 | e |
8092
| exception-xss.js:15:9:15:11 | foo | exception-xss.js:4:20:4:20 | x |
8193
| exception-xss.js:16:10:16:10 | e | exception-xss.js:17:18:17:18 | e |
8294
| exception-xss.js:16:10:16:10 | e | exception-xss.js:17:18:17:18 | e |
@@ -93,15 +105,24 @@ edges
93105
| exception-xss.js:34:10:34:10 | e | exception-xss.js:35:18:35:18 | e |
94106
| exception-xss.js:34:10:34:10 | e | exception-xss.js:35:18:35:18 | e |
95107
| exception-xss.js:38:16:38:16 | x | exception-xss.js:39:9:39:9 | x |
108+
| exception-xss.js:39:3:39:10 | exceptional return of deep2(x) | exception-xss.js:46:3:46:19 | exceptional return of deep("bar" + foo) |
109+
| exception-xss.js:39:3:39:10 | exceptional return of deep2(x) | exception-xss.js:46:3:46:19 | exceptional return of deep("bar" + foo) |
96110
| exception-xss.js:39:9:39:9 | x | exception-xss.js:41:17:41:17 | x |
97111
| exception-xss.js:41:17:41:17 | x | exception-xss.js:42:9:42:9 | x |
112+
| exception-xss.js:42:3:42:10 | exceptional return of inner(x) | exception-xss.js:39:3:39:10 | exceptional return of deep2(x) |
98113
| exception-xss.js:42:9:42:9 | x | exception-xss.js:4:20:4:20 | x |
114+
| exception-xss.js:46:3:46:19 | exceptional return of deep("bar" + foo) | exception-xss.js:47:10:47:10 | e |
115+
| exception-xss.js:46:3:46:19 | exceptional return of deep("bar" + foo) | exception-xss.js:47:10:47:10 | e |
99116
| exception-xss.js:46:8:46:18 | "bar" + foo | exception-xss.js:38:16:38:16 | x |
100117
| exception-xss.js:46:16:46:18 | foo | exception-xss.js:46:8:46:18 | "bar" + foo |
101118
| exception-xss.js:47:10:47:10 | e | exception-xss.js:48:18:48:18 | e |
102119
| exception-xss.js:47:10:47:10 | e | exception-xss.js:48:18:48:18 | e |
103120
| exception-xss.js:74:28:74:28 | x | exception-xss.js:75:10:75:10 | x |
121+
| exception-xss.js:75:4:75:11 | exceptional return of inner(x) | exception-xss.js:81:3:81:19 | exceptional return of myWeirdInner(foo) |
122+
| exception-xss.js:75:4:75:11 | exceptional return of inner(x) | exception-xss.js:81:3:81:19 | exceptional return of myWeirdInner(foo) |
104123
| exception-xss.js:75:10:75:10 | x | exception-xss.js:4:20:4:20 | x |
124+
| exception-xss.js:81:3:81:19 | exceptional return of myWeirdInner(foo) | exception-xss.js:82:10:82:10 | e |
125+
| exception-xss.js:81:3:81:19 | exceptional return of myWeirdInner(foo) | exception-xss.js:82:10:82:10 | e |
105126
| exception-xss.js:81:16:81:18 | foo | exception-xss.js:74:28:74:28 | x |
106127
| exception-xss.js:82:10:82:10 | e | exception-xss.js:83:18:83:18 | e |
107128
| exception-xss.js:82:10:82:10 | e | exception-xss.js:83:18:83:18 | e |
@@ -128,3 +149,4 @@ edges
128149
| exception-xss.js:83:18:83:18 | e | exception-xss.js:2:15:2:31 | document.location | exception-xss.js:83:18:83:18 | e | Cross-site scripting vulnerability due to $@. | exception-xss.js:2:15:2:31 | document.location | user-provided value |
129150
| exception-xss.js:91:18:91:18 | e | exception-xss.js:2:15:2:31 | document.location | exception-xss.js:91:18:91:18 | e | Cross-site scripting vulnerability due to $@. | exception-xss.js:2:15:2:31 | document.location | user-provided value |
130151
| exception-xss.js:97:18:97:18 | e | exception-xss.js:2:15:2:31 | document.location | exception-xss.js:97:18:97:18 | e | Cross-site scripting vulnerability due to $@. | exception-xss.js:2:15:2:31 | document.location | user-provided value |
152+
| exception-xss.js:109:14:109:30 | "Exception: " + e | exception-xss.js:107:13:107:25 | req.params.id | exception-xss.js:109:14:109:30 | "Exception: " + e | Cross-site scripting vulnerability due to $@. | exception-xss.js:107:13:107:25 | req.params.id | user-provided value |

0 commit comments

Comments
 (0)