Skip to content

Commit 38f763d

Browse files
codeql-cicodeql-ci
authored
Merge pull request github#6192 from asgerf/js/string-literals-as-source-nodes
Approved by esbena
2 parents 6823855 + c3b7d85 commit 38f763d

File tree

6 files changed

+29
-3
lines changed

6 files changed

+29
-3
lines changed

‎javascript/ql/src/semmle/javascript/dataflow/Sources.qll

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -321,7 +321,8 @@ module SourceNode {
321321
astNode instanceof ImportMetaExpr or
322322
astNode instanceof TaggedTemplateExpr or
323323
astNode instanceof Angular2::PipeRefExpr or
324-
astNode instanceof Angular2::TemplateVarRefExpr
324+
astNode instanceof Angular2::TemplateVarRefExpr or
325+
astNode instanceof StringLiteral
325326
)
326327
or
327328
DataFlow::parameterNode(this, _)

‎javascript/ql/src/semmle/javascript/frameworks/React.qll

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,11 @@ abstract class ReactComponent extends ASTNode {
248248
* Holds if `f` always returns a JSX element or fragment, or a React element.
249249
*/
250250
private predicate alwaysReturnsJSXOrReactElements(Function f) {
251-
forex(Expr e | e.flow().(DataFlow::SourceNode).flowsToExpr(f.getAReturnedExpr()) |
251+
forex(Expr e |
252+
e.flow().(DataFlow::SourceNode).flowsToExpr(f.getAReturnedExpr()) and
253+
// Allow returning string constants in addition to JSX/React elemnts.
254+
not exists(e.getStringValue())
255+
|
252256
e instanceof JSXNode or
253257
e instanceof ReactElementDefinition
254258
)

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,10 @@ module DomBasedXss {
331331
* A write to the `template` option of a Vue instance, viewed as an XSS sink.
332332
*/
333333
class VueTemplateSink extends DomBasedXss::Sink {
334-
VueTemplateSink() { this = any(Vue::Instance i).getTemplate() }
334+
VueTemplateSink() {
335+
// Note: don't use Vue::Instance#getTemplate as it includes an unwanted getALocalSource() step
336+
this = any(Vue::Instance i).getOption("template")
337+
}
335338
}
336339

337340
/**

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1439,6 +1439,7 @@ sources
14391439
| eval.js:1:1:5:1 | return of function k |
14401440
| eval.js:3:3:3:6 | eval |
14411441
| eval.js:3:3:3:16 | eval("x = 23") |
1442+
| eval.js:3:8:3:15 | "x = 23" |
14421443
| file://:0:0:0:0 | global access path |
14431444
| sources.js:1:1:1:0 | this |
14441445
| sources.js:1:1:1:12 | new (x => x) |
@@ -1475,6 +1476,8 @@ sources
14751476
| tst.js:1:1:1:0 | this |
14761477
| tst.js:1:1:1:24 | import ... m 'fs'; |
14771478
| tst.js:1:10:1:11 | fs |
1479+
| tst.js:1:20:1:23 | 'fs' |
1480+
| tst.js:4:9:4:12 | "hi" |
14781481
| tst.js:16:1:20:9 | (functi ... ("arg") |
14791482
| tst.js:16:2:16:1 | this |
14801483
| tst.js:16:2:20:1 | functio ... n "";\\n} |
@@ -1483,6 +1486,8 @@ sources
14831486
| tst.js:17:7:17:10 | Math |
14841487
| tst.js:17:7:17:17 | Math.random |
14851488
| tst.js:17:7:17:19 | Math.random() |
1489+
| tst.js:19:10:19:11 | "" |
1490+
| tst.js:20:4:20:8 | "arg" |
14861491
| tst.js:22:7:22:18 | readFileSync |
14871492
| tst.js:28:1:30:3 | (() =>\\n ... les\\n)() |
14881493
| tst.js:28:2:29:3 | () =>\\n x |
@@ -1500,6 +1505,7 @@ sources
15001505
| tst.js:44:1:44:3 | o.m |
15011506
| tst.js:44:1:44:5 | o.m() |
15021507
| tst.js:46:1:46:6 | global |
1508+
| tst.js:46:10:46:11 | "" |
15031509
| tst.js:47:1:47:6 | global |
15041510
| tst.js:49:1:54:1 | class A ... `\\n }\\n} |
15051511
| tst.js:49:17:49:17 | B |
@@ -1527,6 +1533,7 @@ sources
15271533
| tst.js:72:9:72:9 | p |
15281534
| tst.js:72:9:72:11 | p() |
15291535
| tst.js:75:9:75:21 | import('foo') |
1536+
| tst.js:75:16:75:20 | 'foo' |
15301537
| tst.js:80:10:80:10 | v |
15311538
| tst.js:83:11:83:28 | [ for (v of o) v ] |
15321539
| tst.js:85:11:85:28 | ( for (v of o) v ) |

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -294,9 +294,13 @@ typetrack
294294
| flow2.js:18:33:18:79 | Promise ... urce)]) | flow2.js:18:45:18:78 | ["clean ... ource)] | copy $PromiseResolveField$ |
295295
| flow2.js:18:33:18:79 | Promise ... urce)]) | flow2.js:18:45:18:78 | ["clean ... ource)] | store $PromiseResolveField$ |
296296
| flow2.js:22:17:22:70 | await P ... urce)]) | flow2.js:22:23:22:70 | Promise ... urce)]) | load $PromiseResolveField$ |
297+
| flow2.js:22:23:22:70 | Promise ... urce)]) | flow2.js:22:37:22:43 | "clean" | copy $PromiseResolveField$ |
298+
| flow2.js:22:23:22:70 | Promise ... urce)]) | flow2.js:22:37:22:43 | "clean" | store $PromiseResolveField$ |
297299
| flow2.js:22:23:22:70 | Promise ... urce)]) | flow2.js:22:46:22:68 | Promise ... source) | copy $PromiseResolveField$ |
298300
| flow2.js:22:23:22:70 | Promise ... urce)]) | flow2.js:22:46:22:68 | Promise ... source) | store $PromiseResolveField$ |
299301
| flow2.js:25:17:25:69 | await P ... urce)]) | flow2.js:25:23:25:69 | Promise ... urce)]) | load $PromiseResolveField$ |
302+
| flow2.js:25:23:25:69 | Promise ... urce)]) | flow2.js:25:36:25:42 | "clean" | copy $PromiseResolveField$ |
303+
| flow2.js:25:23:25:69 | Promise ... urce)]) | flow2.js:25:36:25:42 | "clean" | store $PromiseResolveField$ |
300304
| flow2.js:25:23:25:69 | Promise ... urce)]) | flow2.js:25:45:25:67 | Promise ... source) | copy $PromiseResolveField$ |
301305
| flow2.js:25:23:25:69 | Promise ... urce)]) | flow2.js:25:45:25:67 | Promise ... source) | store $PromiseResolveField$ |
302306
| flow.js:20:2:20:43 | Promise ... ink(x)) | flow.js:20:36:20:42 | sink(x) | copy $PromiseResolveField$ |
@@ -315,10 +319,14 @@ typetrack
315319
| flow.js:26:2:26:81 | new Pro ... ink(y)) | flow.js:26:74:26:80 | sink(y) | copy $PromiseResolveField$ |
316320
| flow.js:26:2:26:81 | new Pro ... ink(y)) | flow.js:26:74:26:80 | sink(y) | store $PromiseResolveField$ |
317321
| flow.js:26:56:26:56 | x | flow.js:26:2:26:49 | new Pro ... ource)) | load $PromiseResolveField$ |
322+
| flow.js:28:2:28:23 | Promise ... ("foo") | flow.js:28:18:28:22 | "foo" | copy $PromiseResolveField$ |
323+
| flow.js:28:2:28:23 | Promise ... ("foo") | flow.js:28:18:28:22 | "foo" | store $PromiseResolveField$ |
318324
| flow.js:28:2:28:60 | Promise ... ink(z)) | flow.js:28:53:28:59 | sink(z) | copy $PromiseResolveField$ |
319325
| flow.js:28:2:28:60 | Promise ... ink(z)) | flow.js:28:53:28:59 | sink(z) | store $PromiseResolveField$ |
320326
| flow.js:28:30:28:30 | x | flow.js:28:2:28:23 | Promise ... ("foo") | load $PromiseResolveField$ |
321327
| flow.js:28:48:28:48 | z | flow.js:28:2:28:41 | Promise ... source) | load $PromiseResolveField$ |
328+
| flow.js:30:2:30:41 | Promise ... "foo") | flow.js:30:36:30:40 | "foo" | copy $PromiseResolveField$ |
329+
| flow.js:30:2:30:41 | Promise ... "foo") | flow.js:30:36:30:40 | "foo" | store $PromiseResolveField$ |
322330
| flow.js:30:2:30:60 | Promise ... ink(z)) | flow.js:30:53:30:59 | sink(z) | copy $PromiseResolveField$ |
323331
| flow.js:30:2:30:60 | Promise ... ink(z)) | flow.js:30:53:30:59 | sink(z) | store $PromiseResolveField$ |
324332
| flow.js:30:31:30:31 | x | flow.js:30:2:30:24 | Promise ... source) | load $PromiseResolveField$ |
@@ -446,6 +454,8 @@ typetrack
446454
| promises.js:119:3:119:25 | Promise ... source) | promises.js:119:19:119:24 | source | store $PromiseResolveField$ |
447455
| promises.js:125:20:125:39 | when.resolve(source) | promises.js:125:33:125:38 | source | copy $PromiseResolveField$ |
448456
| promises.js:125:20:125:39 | when.resolve(source) | promises.js:125:33:125:38 | source | store $PromiseResolveField$ |
457+
| promises.js:130:14:130:69 | new Pro ... s'); }) | promises.js:130:55:130:64 | 'unicorns' | copy $PromiseResolveField$ |
458+
| promises.js:130:14:130:69 | new Pro ... s'); }) | promises.js:130:55:130:64 | 'unicorns' | store $PromiseResolveField$ |
449459
| promises.js:135:3:137:4 | new Pro ... );\\n }) | promises.js:136:13:136:16 | data | copy $PromiseResolveField$ |
450460
| promises.js:135:3:137:4 | new Pro ... );\\n }) | promises.js:136:13:136:16 | data | store $PromiseResolveField$ |
451461
| promises.js:143:17:143:50 | Synchro ... source) | promises.js:143:44:143:49 | source | copy $PromiseResolveField$ |

‎javascript/ql/test/library-tests/frameworks/Express/tests.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1902,6 +1902,7 @@ test_RouteSetup_getARouteHandler
19021902
| src/advanced-routehandler-registration.js:135:2:135:53 | app.get ... andler) | src/advanced-routehandler-registration.js:135:31:135:52 | dynamic ... handler |
19031903
| src/advanced-routehandler-registration.js:139:1:139:58 | app.get ... andler) | src/advanced-routehandler-registration.js:139:9:139:30 | bulkReq ... ky.path |
19041904
| src/advanced-routehandler-registration.js:139:1:139:58 | app.get ... andler) | src/advanced-routehandler-registration.js:139:33:139:57 | bulkReq ... handler |
1905+
| src/advanced-routehandler-registration.js:139:1:139:58 | app.get ... andler) | src/controllers/handler-in-bulk-require.js:1:26:1:32 | "bulky" |
19051906
| src/advanced-routehandler-registration.js:139:1:139:58 | app.get ... andler) | src/controllers/handler-in-bulk-require.js:1:44:1:66 | (req, r ... defined |
19061907
| src/advanced-routehandler-registration.js:147:1:147:37 | app.use ... (data)) | src/advanced-routehandler-registration.js:146:28:146:50 | (req, r ... defined |
19071908
| src/advanced-routehandler-registration.js:147:1:147:37 | app.use ... (data)) | src/advanced-routehandler-registration.js:147:9:147:25 | handlers.handlerA |

0 commit comments

Comments
 (0)