Skip to content

Commit cb86687

Browse files
semmle-qlcisemmle-qlci
authored
Merge pull request #1078 from psygnisfive/UndefinedReturns
Approved by xiemaisi
2 parents 5d9d23e + f368379 commit cb86687

File tree

5 files changed

+184
-0
lines changed

5 files changed

+184
-0
lines changed

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

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,50 @@ class Function extends @function, Parameterized, TypeParameterized, StmtContaine
163163
result = getAReturnStmt().getExpr()
164164
}
165165

166+
/**
167+
* Gets a return from a function which has undefined value (that is, implicit
168+
* returns and returns without expressions).
169+
*
170+
* Functions can have undefined returns in a few different ways:
171+
*
172+
* 1. An explicit return statement with no expression (the statement `return;`)
173+
*
174+
* 2. An implicit return resulting from an expression executing as the last thing
175+
* in the function. For example, the test in a final `if` statement:
176+
*
177+
* ```
178+
* function foo() {
179+
* ...
180+
* if (test) { return 1; }
181+
* }
182+
* ```
183+
*
184+
* Some things look like they might return undefined but actually don't because
185+
* the containing functioning doesn't return at all. For instance, `throw`
186+
* statements prevent the containing function from returning, so they don't count
187+
* as undefined returns. Similarly, `yield` doesn't actually cause a return,
188+
* since the containing function is a generator and can be re-entered, so we also
189+
* exclude yields entirely. Likewise, we exclude generator functions from
190+
* consideration, as well as asynchronous functions, since calls to both produce
191+
* something distinct from what's explicitly returned by the function.
192+
*
193+
* Despite the fact that yield expressions are invalid outside of generators, we
194+
* include them anyway just to ensure that we're not relying on a perfect analysis
195+
* of a function to be a generator, and instead are looking also explicitly at the
196+
* return sites.
197+
*/
198+
199+
ConcreteControlFlowNode getAnUndefinedReturn() {
200+
not this.getBody() instanceof Expr and
201+
not this.isGenerator() and
202+
not this.isAsync() and
203+
not result instanceof ThrowStmt and
204+
not result instanceof YieldExpr and
205+
not (result instanceof ReturnStmt and exists(result.(ReturnStmt).getExpr())) and
206+
result.getContainer() = this and
207+
result.isAFinalNode()
208+
}
209+
166210
/**
167211
* Gets the function whose `this` binding a `this` expression in this function refers to,
168212
* which is the nearest enclosing non-arrow function.

‎javascript/ql/test/library-tests/Functions/getAnUndefinedReturn.qll‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
import javascript
2+
import semmle.javascript.CFG
3+
4+
query predicate test_getAnUndefinedReturn(Function fun, ConcreteControlFlowNode final) {
5+
final = fun.getAnUndefinedReturn()
6+
}

‎javascript/ql/test/library-tests/Functions/tests.expected‎

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,14 @@ test_getVariable
1212
| tst.js:11:1:11:35 | functio ... ts; } } |
1313
| tst.js:12:1:12:44 | functio ... s) {} } |
1414
| tst.js:14:1:14:37 | functio ... s[0]; } |
15+
| undefinedreturns.js:13:1:13:28 | functio ... n() { } |
16+
| undefinedreturns.js:14:1:14:29 | async f ... n() { } |
17+
| undefinedreturns.js:15:1:15:40 | functio ... ow 1; } |
18+
| undefinedreturns.js:16:1:16:41 | functio ... ld 1; } |
19+
| undefinedreturns.js:17:1:17:49 | functio ... rn 1; } |
20+
| undefinedreturns.js:27:1:27:30 | functio ... y() { } |
21+
| undefinedreturns.js:28:1:28:48 | functio ... turn; } |
22+
| undefinedreturns.js:29:1:29:54 | functio ... 1; } } |
1523
test_getScope
1624
| arrowfns.js:1:24:1:36 | s => s.length |
1725
| arrowfns.js:2:13:2:23 | () => ++cnt |
@@ -33,6 +41,17 @@ test_getScope
3341
| tst.js:11:1:11:35 | functio ... ts; } } |
3442
| tst.js:12:1:12:44 | functio ... s) {} } |
3543
| tst.js:14:1:14:37 | functio ... s[0]; } |
44+
| undefinedreturns.js:11:20:11:32 | function () 1 |
45+
| undefinedreturns.js:12:29:12:35 | () => 1 |
46+
| undefinedreturns.js:13:1:13:28 | functio ... n() { } |
47+
| undefinedreturns.js:14:1:14:29 | async f ... n() { } |
48+
| undefinedreturns.js:15:1:15:40 | functio ... ow 1; } |
49+
| undefinedreturns.js:16:1:16:41 | functio ... ld 1; } |
50+
| undefinedreturns.js:17:1:17:49 | functio ... rn 1; } |
51+
| undefinedreturns.js:27:1:27:30 | functio ... y() { } |
52+
| undefinedreturns.js:28:1:28:48 | functio ... turn; } |
53+
| undefinedreturns.js:29:1:29:54 | functio ... 1; } } |
54+
| undefinedreturns.js:30:29:30:37 | () => { } |
3655
test_getParameter
3756
| arrowfns.js:1:24:1:36 | s => s.length | 0 | arrowfns.js:1:24:1:24 | s |
3857
| defaultargs.js:1:1:1:24 | functio ... +19) {} | 0 | defaultargs.js:1:12:1:12 | x |
@@ -52,6 +71,10 @@ test_ReturnedExpression
5271
| arrowfns.js:2:13:2:23 | () => ++cnt | arrowfns.js:2:19:2:23 | ++cnt |
5372
| exprclosures.js:1:7:1:21 | function(x) x+1 | exprclosures.js:1:19:1:21 | x+1 |
5473
| tst.js:14:1:14:37 | functio ... s[0]; } | tst.js:14:23:14:34 | arguments[0] |
74+
| undefinedreturns.js:11:20:11:32 | function () 1 | undefinedreturns.js:11:32:11:32 | 1 |
75+
| undefinedreturns.js:12:29:12:35 | () => 1 | undefinedreturns.js:12:35:12:35 | 1 |
76+
| undefinedreturns.js:17:1:17:49 | functio ... rn 1; } | undefinedreturns.js:17:46:17:46 | 1 |
77+
| undefinedreturns.js:29:1:29:54 | functio ... 1; } } | undefinedreturns.js:29:49:29:49 | 1 |
5578
test_getDefaultArguments
5679
| defaultargs.js:1:15:1:15 | y | defaultargs.js:1:17:1:20 | x+19 |
5780
test_Function
@@ -75,6 +98,17 @@ test_Function
7598
| tst.js:11:1:11:35 | functio ... ts; } } |
7699
| tst.js:12:1:12:44 | functio ... s) {} } |
77100
| tst.js:14:1:14:37 | functio ... s[0]; } |
101+
| undefinedreturns.js:11:20:11:32 | function () 1 |
102+
| undefinedreturns.js:12:29:12:35 | () => 1 |
103+
| undefinedreturns.js:13:1:13:28 | functio ... n() { } |
104+
| undefinedreturns.js:14:1:14:29 | async f ... n() { } |
105+
| undefinedreturns.js:15:1:15:40 | functio ... ow 1; } |
106+
| undefinedreturns.js:16:1:16:41 | functio ... ld 1; } |
107+
| undefinedreturns.js:17:1:17:49 | functio ... rn 1; } |
108+
| undefinedreturns.js:27:1:27:30 | functio ... y() { } |
109+
| undefinedreturns.js:28:1:28:48 | functio ... turn; } |
110+
| undefinedreturns.js:29:1:29:54 | functio ... 1; } } |
111+
| undefinedreturns.js:30:29:30:37 | () => { } |
78112
test_getBody
79113
| arrowfns.js:1:24:1:36 | s => s.length | arrowfns.js:1:29:1:36 | s.length |
80114
| arrowfns.js:2:13:2:23 | () => ++cnt | arrowfns.js:2:19:2:23 | ++cnt |
@@ -96,6 +130,17 @@ test_getBody
96130
| tst.js:11:1:11:35 | functio ... ts; } } | tst.js:11:14:11:35 | { { var ... ts; } } |
97131
| tst.js:12:1:12:44 | functio ... s) {} } | tst.js:12:14:12:44 | { try { ... s) {} } |
98132
| tst.js:14:1:14:37 | functio ... s[0]; } | tst.js:14:14:14:37 | { retur ... s[0]; } |
133+
| undefinedreturns.js:11:20:11:32 | function () 1 | undefinedreturns.js:11:32:11:32 | 1 |
134+
| undefinedreturns.js:12:29:12:35 | () => 1 | undefinedreturns.js:12:35:12:35 | 1 |
135+
| undefinedreturns.js:13:1:13:28 | functio ... n() { } | undefinedreturns.js:13:26:13:28 | { } |
136+
| undefinedreturns.js:14:1:14:29 | async f ... n() { } | undefinedreturns.js:14:27:14:29 | { } |
137+
| undefinedreturns.js:15:1:15:40 | functio ... ow 1; } | undefinedreturns.js:15:29:15:40 | { throw 1; } |
138+
| undefinedreturns.js:16:1:16:41 | functio ... ld 1; } | undefinedreturns.js:16:30:16:41 | { yield 1; } |
139+
| undefinedreturns.js:17:1:17:49 | functio ... rn 1; } | undefinedreturns.js:17:37:17:49 | { return 1; } |
140+
| undefinedreturns.js:27:1:27:30 | functio ... y() { } | undefinedreturns.js:27:28:27:30 | { } |
141+
| undefinedreturns.js:28:1:28:48 | functio ... turn; } | undefinedreturns.js:28:38:28:48 | { return; } |
142+
| undefinedreturns.js:29:1:29:54 | functio ... 1; } } | undefinedreturns.js:29:28:29:54 | { if (t ... 1; } } |
143+
| undefinedreturns.js:30:29:30:37 | () => { } | undefinedreturns.js:30:35:30:37 | { } |
99144
test_getId
100145
| defaultargs.js:1:1:1:24 | functio ... +19) {} | defaultargs.js:1:10:1:10 | f | f |
101146
| generators.js:1:1:4:1 | functio ... i++;\\n} | generators.js:1:11:1:13 | foo | foo |
@@ -110,6 +155,14 @@ test_getId
110155
| tst.js:11:1:11:35 | functio ... ts; } } | tst.js:11:10:11:10 | m | m |
111156
| tst.js:12:1:12:44 | functio ... s) {} } | tst.js:12:10:12:10 | n | n |
112157
| tst.js:14:1:14:37 | functio ... s[0]; } | tst.js:14:10:14:10 | p | p |
158+
| undefinedreturns.js:13:1:13:28 | functio ... n() { } | undefinedreturns.js:13:11:13:22 | generator_fn | generator_fn |
159+
| undefinedreturns.js:14:1:14:29 | async f ... n() { } | undefinedreturns.js:14:16:14:23 | async_fn | async_fn |
160+
| undefinedreturns.js:15:1:15:40 | functio ... ow 1; } | undefinedreturns.js:15:10:15:25 | fn_w_final_throw | fn_w_final_throw |
161+
| undefinedreturns.js:16:1:16:41 | functio ... ld 1; } | undefinedreturns.js:16:11:16:26 | fn_w_final_yield | fn_w_final_yield |
162+
| undefinedreturns.js:17:1:17:49 | functio ... rn 1; } | undefinedreturns.js:17:10:17:33 | fn_w_fi ... _w_expr | fn_w_final_return_w_expr |
163+
| undefinedreturns.js:27:1:27:30 | functio ... y() { } | undefinedreturns.js:27:10:27:24 | fn_w_empty_body | fn_w_empty_body |
164+
| undefinedreturns.js:28:1:28:48 | functio ... turn; } | undefinedreturns.js:28:10:28:34 | fn_w_fi ... wo_expr | fn_w_final_return_wo_expr |
165+
| undefinedreturns.js:29:1:29:54 | functio ... 1; } } | undefinedreturns.js:29:10:29:24 | fn_w_final_expr | fn_w_final_expr |
113166
test_hasRestParameter
114167
| restparms.js:1:1:2:1 | functio ... ys) {\\n} |
115168
test_getArgumentsVariable
@@ -130,6 +183,15 @@ test_getArgumentsVariable
130183
| tst.js:11:1:11:35 | functio ... ts; } } |
131184
| tst.js:12:1:12:44 | functio ... s) {} } |
132185
| tst.js:14:1:14:37 | functio ... s[0]; } |
186+
| undefinedreturns.js:11:20:11:32 | function () 1 |
187+
| undefinedreturns.js:13:1:13:28 | functio ... n() { } |
188+
| undefinedreturns.js:14:1:14:29 | async f ... n() { } |
189+
| undefinedreturns.js:15:1:15:40 | functio ... ow 1; } |
190+
| undefinedreturns.js:16:1:16:41 | functio ... ld 1; } |
191+
| undefinedreturns.js:17:1:17:49 | functio ... rn 1; } |
192+
| undefinedreturns.js:27:1:27:30 | functio ... y() { } |
193+
| undefinedreturns.js:28:1:28:48 | functio ... turn; } |
194+
| undefinedreturns.js:29:1:29:54 | functio ... 1; } } |
133195
test_getBodyStmt
134196
| arrowfns.js:3:12:3:41 | () => { ... "); ; } | 0 | arrowfns.js:3:20:3:37 | alert("Wake up!"); |
135197
| arrowfns.js:3:12:3:41 | () => { ... "); ; } | 1 | arrowfns.js:3:39:3:39 | ; |
@@ -138,9 +200,16 @@ test_getBodyStmt
138200
| tst.js:11:1:11:35 | functio ... ts; } } | 0 | tst.js:11:16:11:33 | { var arguments; } |
139201
| tst.js:12:1:12:44 | functio ... s) {} } | 0 | tst.js:12:16:12:42 | try { } ... nts) {} |
140202
| tst.js:14:1:14:37 | functio ... s[0]; } | 0 | tst.js:14:16:14:35 | return arguments[0]; |
203+
| undefinedreturns.js:15:1:15:40 | functio ... ow 1; } | 0 | undefinedreturns.js:15:31:15:38 | throw 1; |
204+
| undefinedreturns.js:16:1:16:41 | functio ... ld 1; } | 0 | undefinedreturns.js:16:32:16:39 | yield 1; |
205+
| undefinedreturns.js:17:1:17:49 | functio ... rn 1; } | 0 | undefinedreturns.js:17:39:17:47 | return 1; |
206+
| undefinedreturns.js:28:1:28:48 | functio ... turn; } | 0 | undefinedreturns.js:28:40:28:46 | return; |
207+
| undefinedreturns.js:29:1:29:54 | functio ... 1; } } | 0 | undefinedreturns.js:29:30:29:52 | if (tes ... rn 1; } |
141208
test_isGenerator
142209
| generators.js:1:1:4:1 | functio ... i++;\\n} |
143210
| generators.js:6:2:6:19 | function* bar() {} |
211+
| undefinedreturns.js:13:1:13:28 | functio ... n() { } |
212+
| undefinedreturns.js:16:1:16:41 | functio ... ld 1; } |
144213
test_usesArgumentsObject
145214
| tst.js:14:1:14:37 | functio ... s[0]; } |
146215
test_getEnclosingStmt
@@ -164,7 +233,41 @@ test_getEnclosingStmt
164233
| tst.js:11:1:11:35 | functio ... ts; } } | tst.js:11:1:11:35 | functio ... ts; } } |
165234
| tst.js:12:1:12:44 | functio ... s) {} } | tst.js:12:1:12:44 | functio ... s) {} } |
166235
| tst.js:14:1:14:37 | functio ... s[0]; } | tst.js:14:1:14:37 | functio ... s[0]; } |
236+
| undefinedreturns.js:11:20:11:32 | function () 1 | undefinedreturns.js:11:1:11:33 | const f ... n () 1; |
237+
| undefinedreturns.js:12:29:12:35 | () => 1 | undefinedreturns.js:12:1:12:36 | const a ... ) => 1; |
238+
| undefinedreturns.js:13:1:13:28 | functio ... n() { } | undefinedreturns.js:13:1:13:28 | functio ... n() { } |
239+
| undefinedreturns.js:14:1:14:29 | async f ... n() { } | undefinedreturns.js:14:1:14:29 | async f ... n() { } |
240+
| undefinedreturns.js:15:1:15:40 | functio ... ow 1; } | undefinedreturns.js:15:1:15:40 | functio ... ow 1; } |
241+
| undefinedreturns.js:16:1:16:41 | functio ... ld 1; } | undefinedreturns.js:16:1:16:41 | functio ... ld 1; } |
242+
| undefinedreturns.js:17:1:17:49 | functio ... rn 1; } | undefinedreturns.js:17:1:17:49 | functio ... rn 1; } |
243+
| undefinedreturns.js:27:1:27:30 | functio ... y() { } | undefinedreturns.js:27:1:27:30 | functio ... y() { } |
244+
| undefinedreturns.js:28:1:28:48 | functio ... turn; } | undefinedreturns.js:28:1:28:48 | functio ... turn; } |
245+
| undefinedreturns.js:29:1:29:54 | functio ... 1; } } | undefinedreturns.js:29:1:29:54 | functio ... 1; } } |
246+
| undefinedreturns.js:30:29:30:37 | () => { } | undefinedreturns.js:30:1:30:38 | const a ... => { }; |
167247
test_isRestParameter
168248
| restparms.js:1:18:1:19 | ys |
169249
test_ReturnStmt
170250
| tst.js:14:1:14:37 | functio ... s[0]; } | tst.js:14:16:14:35 | return arguments[0]; |
251+
| undefinedreturns.js:17:1:17:49 | functio ... rn 1; } | undefinedreturns.js:17:39:17:47 | return 1; |
252+
| undefinedreturns.js:28:1:28:48 | functio ... turn; } | undefinedreturns.js:28:40:28:46 | return; |
253+
| undefinedreturns.js:29:1:29:54 | functio ... 1; } } | undefinedreturns.js:29:42:29:50 | return 1; |
254+
test_getAnUndefinedReturn
255+
| arrowfns.js:3:12:3:41 | () => { ... "); ; } | arrowfns.js:3:39:3:39 | ; |
256+
| defaultargs.js:1:1:1:24 | functio ... +19) {} | defaultargs.js:1:23:1:24 | {} |
257+
| restparms.js:1:1:2:1 | functio ... ys) {\\n} | restparms.js:1:22:2:1 | {\\n} |
258+
| tst.js:1:1:1:15 | function A() {} | tst.js:1:14:1:15 | {} |
259+
| tst.js:2:1:2:16 | function B(x) {} | tst.js:2:15:2:16 | {} |
260+
| tst.js:3:1:3:19 | function C(x, y) {} | tst.js:3:18:3:19 | {} |
261+
| tst.js:4:9:4:21 | function() {} | tst.js:4:20:4:21 | {} |
262+
| tst.js:5:2:5:15 | function(x) {} | tst.js:5:14:5:15 | {} |
263+
| tst.js:6:2:6:18 | function(x, y) {} | tst.js:6:17:6:18 | {} |
264+
| tst.js:7:9:7:23 | function h() {} | tst.js:7:22:7:23 | {} |
265+
| tst.js:9:1:9:24 | functio ... nts) {} | tst.js:9:23:9:24 | {} |
266+
| tst.js:10:1:10:31 | functio ... ents; } | tst.js:10:20:10:28 | arguments |
267+
| tst.js:11:1:11:35 | functio ... ts; } } | tst.js:11:22:11:30 | arguments |
268+
| tst.js:12:1:12:44 | functio ... s) {} } | tst.js:12:20:12:22 | { } |
269+
| tst.js:12:1:12:44 | functio ... s) {} } | tst.js:12:41:12:42 | {} |
270+
| undefinedreturns.js:27:1:27:30 | functio ... y() { } | undefinedreturns.js:27:28:27:30 | { } |
271+
| undefinedreturns.js:28:1:28:48 | functio ... turn; } | undefinedreturns.js:28:40:28:46 | return; |
272+
| undefinedreturns.js:29:1:29:54 | functio ... 1; } } | undefinedreturns.js:29:34:29:37 | test |
273+
| undefinedreturns.js:30:29:30:37 | () => { } | undefinedreturns.js:30:35:30:37 | { } |

‎javascript/ql/test/library-tests/Functions/tests.ql‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,3 +14,4 @@ import usesArgumentsObject
1414
import getEnclosingStmt
1515
import isRestParameter
1616
import ReturnStmt
17+
import getAnUndefinedReturn

‎javascript/ql/test/library-tests/Functions/undefinedreturns.js‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
//semmle-extractor-options: --experimental
2+
3+
4+
5+
//////////////////
6+
// //
7+
// DON'T FIND //
8+
// //
9+
//////////////////
10+
11+
const fn_closure = function () 1;
12+
const arrowfn_w_expr_body = () => 1;
13+
function* generator_fn() { }
14+
async function async_fn() { }
15+
function fn_w_final_throw() { throw 1; }
16+
function* fn_w_final_yield() { yield 1; }
17+
function fn_w_final_return_w_expr() { return 1; }
18+
19+
20+
21+
////////////
22+
// //
23+
// FIND //
24+
// //
25+
////////////
26+
27+
function fn_w_empty_body() { }
28+
function fn_w_final_return_wo_expr() { return; }
29+
function fn_w_final_expr() { if (test) { return 1; } }
30+
const arrowfn_w_blockbody = () => { };

0 commit comments

Comments
 (0)