Skip to content

Commit 9fa7393

Browse files
erik-krogherik-krogh
committed
add support for try-statements with no catch block
1 parent 91674f6 commit 9fa7393

File tree

4 files changed

+38
-18
lines changed

4 files changed

+38
-18
lines changed

‎javascript/ql/src/semmle/javascript/Expr.qll‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -251,11 +251,11 @@ class Expr extends @expr, ExprOrStmt, ExprOrType, AST::ValueNode {
251251
* causes an exception to be thrown.
252252
*/
253253
DataFlow::Node getThrowsToNode() {
254-
if exists(this.getEnclosingStmt().getEnclosingTryStmt())
254+
if exists(this.getEnclosingStmt().getEnclosingTryCatchStmt())
255255
then
256256
result = DataFlow::parameterNode(this
257257
.getEnclosingStmt()
258-
.getEnclosingTryStmt()
258+
.getEnclosingTryCatchStmt()
259259
.getACatchClause()
260260
.getAParameter())
261261
else result = any(DataFlow::FunctionNode f | f.getFunction() = this.getContainer()).getExceptionalReturn()

‎javascript/ql/src/semmle/javascript/Stmt.qll‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,12 +57,13 @@ class Stmt extends @stmt, ExprOrStmt, Documentable {
5757
override predicate isAmbient() { hasDeclareKeyword(this) or getParent().isAmbient() }
5858

5959
/**
60-
* Gets the `try` statement containing this statement without crossing function
61-
* boundaries or other `try ` statements.
60+
* Gets the `try` statement with a catch block containing this statement without
61+
* crossing function boundaries or other `try ` statements with catch blocks.
6262
*/
63-
TryStmt getEnclosingTryStmt() {
63+
TryStmt getEnclosingTryCatchStmt() {
6464
getParentStmt+() = result.getBody() and
65-
not exists(TryStmt mid |
65+
exists(result.getACatchClause()) and
66+
not exists(TryStmt mid | exists(mid.getACatchClause()) |
6667
getParentStmt+() = mid.getBody() and mid.getParentStmt+() = result.getBody()
6768
)
6869
}

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

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -47,12 +47,16 @@ nodes
4747
| exception-xss.js:96:10:96:10 | e |
4848
| exception-xss.js:97:18:97:18 | e |
4949
| exception-xss.js:97:18:97:18 | e |
50-
| exception-xss.js:107:13:107:25 | req.params.id |
51-
| exception-xss.js:107:13:107:25 | req.params.id |
52-
| exception-xss.js:108:11:108:11 | e |
53-
| exception-xss.js:109:14:109:30 | "Exception: " + e |
54-
| exception-xss.js:109:14:109:30 | "Exception: " + e |
55-
| exception-xss.js:109:30:109:30 | e |
50+
| exception-xss.js:102:12:102:14 | foo |
51+
| exception-xss.js:106:10:106:10 | e |
52+
| exception-xss.js:107:18:107:18 | e |
53+
| exception-xss.js:107:18:107:18 | e |
54+
| exception-xss.js:117:13:117:25 | req.params.id |
55+
| exception-xss.js:117:13:117:25 | req.params.id |
56+
| exception-xss.js:118:11:118:11 | e |
57+
| exception-xss.js:119:14:119:30 | "Exception: " + e |
58+
| exception-xss.js:119:14:119:30 | "Exception: " + e |
59+
| exception-xss.js:119:30:119:30 | e |
5660
edges
5761
| exception-xss.js:2:9:2:31 | foo | exception-xss.js:9:11:9:13 | foo |
5862
| exception-xss.js:2:9:2:31 | foo | exception-xss.js:15:9:15:11 | foo |
@@ -63,6 +67,7 @@ edges
6367
| exception-xss.js:2:9:2:31 | foo | exception-xss.js:81:16:81:18 | foo |
6468
| exception-xss.js:2:9:2:31 | foo | exception-xss.js:89:11:89:13 | foo |
6569
| exception-xss.js:2:9:2:31 | foo | exception-xss.js:95:12:95:14 | foo |
70+
| exception-xss.js:2:9:2:31 | foo | exception-xss.js:102:12:102:14 | foo |
6671
| exception-xss.js:2:15:2:31 | document.location | exception-xss.js:2:9:2:31 | foo |
6772
| exception-xss.js:2:15:2:31 | document.location | exception-xss.js:2:9:2:31 | foo |
6873
| exception-xss.js:9:11:9:13 | foo | exception-xss.js:10:10:10:10 | e |
@@ -101,11 +106,14 @@ edges
101106
| exception-xss.js:95:12:95:14 | foo | exception-xss.js:95:11:95:22 | [foo, "bar"] |
102107
| exception-xss.js:96:10:96:10 | e | exception-xss.js:97:18:97:18 | e |
103108
| exception-xss.js:96:10:96:10 | e | exception-xss.js:97:18:97:18 | e |
104-
| exception-xss.js:107:13:107:25 | req.params.id | exception-xss.js:108:11:108:11 | e |
105-
| exception-xss.js:107:13:107:25 | req.params.id | exception-xss.js:108:11:108:11 | e |
106-
| exception-xss.js:108:11:108:11 | e | exception-xss.js:109:30:109:30 | e |
107-
| exception-xss.js:109:30:109:30 | e | exception-xss.js:109:14:109:30 | "Exception: " + e |
108-
| exception-xss.js:109:30:109:30 | e | exception-xss.js:109:14:109:30 | "Exception: " + e |
109+
| exception-xss.js:102:12:102:14 | foo | exception-xss.js:106:10:106:10 | e |
110+
| exception-xss.js:106:10:106:10 | e | exception-xss.js:107:18:107:18 | e |
111+
| exception-xss.js:106:10:106:10 | e | exception-xss.js:107:18:107:18 | e |
112+
| exception-xss.js:117:13:117:25 | req.params.id | exception-xss.js:118:11:118:11 | e |
113+
| exception-xss.js:117:13:117:25 | req.params.id | exception-xss.js:118:11:118:11 | e |
114+
| exception-xss.js:118:11:118:11 | e | exception-xss.js:119:30:119:30 | e |
115+
| exception-xss.js:119:30:119:30 | e | exception-xss.js:119:14:119:30 | "Exception: " + e |
116+
| exception-xss.js:119:30:119:30 | e | exception-xss.js:119:14:119:30 | "Exception: " + e |
109117
#select
110118
| exception-xss.js:11:18:11:18 | e | exception-xss.js:2:15:2:31 | document.location | exception-xss.js:11:18:11:18 | e | Cross-site scripting vulnerability due to $@. | exception-xss.js:2:15:2:31 | document.location | user-provided value |
111119
| exception-xss.js:17:18:17:18 | e | exception-xss.js:2:15:2:31 | document.location | exception-xss.js:17:18:17:18 | e | Cross-site scripting vulnerability due to $@. | exception-xss.js:2:15:2:31 | document.location | user-provided value |
@@ -116,4 +124,5 @@ edges
116124
| 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 |
117125
| 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 |
118126
| 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 |
119-
| 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 |
127+
| exception-xss.js:107:18:107:18 | e | exception-xss.js:2:15:2:31 | document.location | exception-xss.js:107:18:107:18 | e | Cross-site scripting vulnerability due to $@. | exception-xss.js:2:15:2:31 | document.location | user-provided value |
128+
| exception-xss.js:119:14:119:30 | "Exception: " + e | exception-xss.js:117:13:117:25 | req.params.id | exception-xss.js:119:14:119:30 | "Exception: " + e | Cross-site scripting vulnerability due to $@. | exception-xss.js:117:13:117:25 | req.params.id | user-provided value |

‎javascript/ql/test/query-tests/Security/CWE-079/exception-xss.js‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,16 @@
9696
} catch(e) {
9797
$('myId').html(e); // NOT OK!
9898
}
99+
100+
try {
101+
try {
102+
unknown(foo);
103+
} finally {
104+
// nothing
105+
}
106+
} catch(e) {
107+
$('myId').html(e); // NOT OK!
108+
}
99109
});
100110

101111
var express = require('express');

0 commit comments

Comments
 (0)