Skip to content

Commit a43bb1f

Browse files
codeql-cicodeql-ci
authored
Merge pull request github#5499 from asgerf/js/non-recursive-sourcenode
Approved by erik-krogh
2 parents ce63809 + 6c8b4a8 commit a43bb1f

File tree

8 files changed

+107
-80
lines changed

8 files changed

+107
-80
lines changed

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

Lines changed: 27 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -619,11 +619,11 @@ module API {
619619
cached
620620
predicate use(TApiNode nd, DataFlow::Node ref) {
621621
exists(string m, Module mod | nd = MkModuleDef(m) and mod = importableModule(m) |
622-
ref.(ModuleAsSourceNode).getModule() = mod
622+
ref.(ModuleVarNode).getModule() = mod
623623
)
624624
or
625625
exists(string m, Module mod | nd = MkModuleExport(m) and mod = importableModule(m) |
626-
ref.(ExportsAsSourceNode).getModule() = mod
626+
ref.(ExportsVarNode).getModule() = mod
627627
or
628628
exists(DataFlow::Node base | use(MkModuleDef(m), base) |
629629
ref = trackUseNode(base).getAPropertyRead("exports")
@@ -746,9 +746,9 @@ module API {
746746
or
747747
// additional backwards step from `require('m')` to `exports` or `module.exports` in m
748748
exists(Import imp | imp.getImportedModuleNode() = trackDefNode(nd, t.continue()) |
749-
result.(ExportsAsSourceNode).getModule() = imp.getImportedModule()
749+
result.(ExportsVarNode).getModule() = imp.getImportedModule()
750750
or
751-
exists(ModuleAsSourceNode mod |
751+
exists(ModuleVarNode mod |
752752
mod.getModule() = imp.getImportedModule() and
753753
result = mod.(DataFlow::SourceNode).getAPropertyRead("exports")
754754
)
@@ -983,31 +983,44 @@ private module Label {
983983
string promised() { result = "promised" }
984984
}
985985

986+
private class NodeModuleSourcesNodes extends DataFlow::SourceNode::Range {
987+
Variable v;
988+
989+
NodeModuleSourcesNodes() {
990+
exists(NodeModule m |
991+
this = DataFlow::ssaDefinitionNode(SSA::implicitInit(v)) and
992+
v = [m.getModuleVariable(), m.getExportsVariable()]
993+
)
994+
}
995+
996+
Variable getVariable() { result = v }
997+
}
998+
986999
/**
987-
* A CommonJS/AMD `module` variable, considered as a source node.
1000+
* A CommonJS/AMD `module` variable.
9881001
*/
989-
private class ModuleAsSourceNode extends DataFlow::SourceNode::Range {
1002+
private class ModuleVarNode extends DataFlow::Node {
9901003
Module m;
9911004

992-
ModuleAsSourceNode() {
993-
this = DataFlow::ssaDefinitionNode(SSA::implicitInit(m.(NodeModule).getModuleVariable()))
1005+
ModuleVarNode() {
1006+
this.(NodeModuleSourcesNodes).getVariable() = m.(NodeModule).getModuleVariable()
9941007
or
995-
this = DataFlow::parameterNode(m.(AmdModule).getDefine().getModuleParameter())
1008+
DataFlow::parameterNode(this, m.(AmdModule).getDefine().getModuleParameter())
9961009
}
9971010

9981011
Module getModule() { result = m }
9991012
}
10001013

10011014
/**
1002-
* A CommonJS/AMD `exports` variable, considered as a source node.
1015+
* A CommonJS/AMD `exports` variable.
10031016
*/
1004-
private class ExportsAsSourceNode extends DataFlow::SourceNode::Range {
1017+
private class ExportsVarNode extends DataFlow::Node {
10051018
Module m;
10061019

1007-
ExportsAsSourceNode() {
1008-
this = DataFlow::ssaDefinitionNode(SSA::implicitInit(m.(NodeModule).getExportsVariable()))
1020+
ExportsVarNode() {
1021+
this.(NodeModuleSourcesNodes).getVariable() = m.(NodeModule).getExportsVariable()
10091022
or
1010-
this = DataFlow::parameterNode(m.(AmdModule).getDefine().getExportsParameter())
1023+
DataFlow::parameterNode(this, m.(AmdModule).getDefine().getExportsParameter())
10111024
}
10121025

10131026
Module getModule() { result = m }

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,11 @@ module DOM {
315315
)
316316
}
317317

318+
private InferredType getArgumentTypeFromJQueryMethodGet(JQuery::MethodCall call) {
319+
call.getMethodName() = "get" and
320+
result = call.getArgument(0).analyze().getAType()
321+
}
322+
318323
private class DefaultRange extends Range {
319324
DefaultRange() {
320325
this.asExpr().(VarAccess).getVariable() instanceof DOMGlobalVariable
@@ -344,7 +349,7 @@ module DOM {
344349
or
345350
exists(JQuery::MethodCall call | this = call and call.getMethodName() = "get" |
346351
call.getNumArgument() = 1 and
347-
unique(InferredType t | t = call.getArgument(0).analyze().getAType()) = TTNumber()
352+
unique(InferredType t | t = getArgumentTypeFromJQueryMethodGet(call)) = TTNumber()
348353
)
349354
or
350355
// A `this` node from a callback given to a `$().each(callback)` call.

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

Lines changed: 42 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -16,51 +16,53 @@ abstract class HtmlSanitizerCall extends DataFlow::CallNode {
1616
abstract DataFlow::Node getInput();
1717
}
1818

19+
pragma[noinline]
20+
private DataFlow::SourceNode htmlSanitizerFunction() {
21+
result = DataFlow::moduleMember("ent", "encode")
22+
or
23+
result = DataFlow::moduleMember("entities", "encodeHTML")
24+
or
25+
result = DataFlow::moduleMember("entities", "encodeXML")
26+
or
27+
result = DataFlow::moduleMember("escape-goat", "escape")
28+
or
29+
result = DataFlow::moduleMember("he", "encode")
30+
or
31+
result = DataFlow::moduleMember("he", "escape")
32+
or
33+
result = DataFlow::moduleImport("sanitize-html")
34+
or
35+
result = DataFlow::moduleMember("sanitizer", "escape")
36+
or
37+
result = DataFlow::moduleMember("sanitizer", "sanitize")
38+
or
39+
result = DataFlow::moduleMember("validator", "escape")
40+
or
41+
result = DataFlow::moduleImport("xss")
42+
or
43+
result = DataFlow::moduleMember("xss-filters", _)
44+
or
45+
result = LodashUnderscore::member("escape")
46+
or
47+
exists(DataFlow::PropRead read | read = result |
48+
read.getPropertyName() = "sanitize" and
49+
read.getBase().asExpr().(VarAccess).getName() = "DOMPurify"
50+
)
51+
or
52+
exists(string name | name = "encode" or name = "encodeNonUTF" |
53+
result = DataFlow::moduleMember("html-entities", _).getAnInstantiation().getAPropertyRead(name) or
54+
result = DataFlow::moduleMember("html-entities", _).getAPropertyRead(name)
55+
)
56+
or
57+
result = Closure::moduleImport("goog.string.htmlEscape")
58+
}
59+
1960
/**
2061
* Matches HTML sanitizers from known NPM packages as well as home-made sanitizers (matched by name).
2162
*/
2263
private class DefaultHtmlSanitizerCall extends HtmlSanitizerCall {
2364
DefaultHtmlSanitizerCall() {
24-
exists(DataFlow::SourceNode callee | this = callee.getACall() |
25-
callee = DataFlow::moduleMember("ent", "encode")
26-
or
27-
callee = DataFlow::moduleMember("entities", "encodeHTML")
28-
or
29-
callee = DataFlow::moduleMember("entities", "encodeXML")
30-
or
31-
callee = DataFlow::moduleMember("escape-goat", "escape")
32-
or
33-
callee = DataFlow::moduleMember("he", "encode")
34-
or
35-
callee = DataFlow::moduleMember("he", "escape")
36-
or
37-
callee = DataFlow::moduleImport("sanitize-html")
38-
or
39-
callee = DataFlow::moduleMember("sanitizer", "escape")
40-
or
41-
callee = DataFlow::moduleMember("sanitizer", "sanitize")
42-
or
43-
callee = DataFlow::moduleMember("validator", "escape")
44-
or
45-
callee = DataFlow::moduleImport("xss")
46-
or
47-
callee = DataFlow::moduleMember("xss-filters", _)
48-
or
49-
callee = LodashUnderscore::member("escape")
50-
or
51-
exists(DataFlow::PropRead read | read = callee |
52-
read.getPropertyName() = "sanitize" and
53-
read.getBase().asExpr().(VarAccess).getName() = "DOMPurify"
54-
)
55-
or
56-
exists(string name | name = "encode" or name = "encodeNonUTF" |
57-
callee =
58-
DataFlow::moduleMember("html-entities", _).getAnInstantiation().getAPropertyRead(name) or
59-
callee = DataFlow::moduleMember("html-entities", _).getAPropertyRead(name)
60-
)
61-
or
62-
callee = Closure::moduleImport("goog.string.htmlEscape")
63-
)
65+
this = htmlSanitizerFunction().getACall()
6466
or
6567
// Match home-made sanitizers by name.
6668
exists(string calleeName | calleeName = getCalleeName() |

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -596,6 +596,7 @@ private module ClosurePromise {
596596
* A promise created by a call `goog.Promise.resolve(value)`.
597597
*/
598598
private class ResolvedClosurePromiseDefinition extends ResolvedPromiseDefinition {
599+
pragma[noinline]
599600
ResolvedClosurePromiseDefinition() {
600601
this = Closure::moduleImport("goog.Promise.resolve").getACall()
601602
}

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

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -55,12 +55,8 @@ module StringConcatenation {
5555
exists(DataFlow::MethodCallNode call |
5656
node = call and
5757
call.getMethodName() = "concat" and
58-
not (
59-
exists(DataFlow::ArrayCreationNode array |
60-
array.flowsTo(call.getAnArgument()) or array.flowsTo(call.getReceiver())
61-
)
62-
or
63-
DataFlow::reflectiveCallNode(_) = call
58+
not exists(DataFlow::ArrayCreationNode array |
59+
array.flowsTo(call.getAnArgument()) or array.flowsTo(call.getReceiver())
6460
) and
6561
(
6662
n = 0 and

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

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -59,18 +59,16 @@ class ParameterNode extends DataFlow::SourceNode {
5959
* ```
6060
*/
6161
class InvokeNode extends DataFlow::SourceNode {
62-
DataFlow::Impl::InvokeNodeDef impl;
63-
64-
InvokeNode() { this = impl }
62+
InvokeNode() { this instanceof DataFlow::Impl::InvokeNodeDef }
6563

6664
/** Gets the syntactic invoke expression underlying this function invocation. */
67-
InvokeExpr getInvokeExpr() { result = impl.getInvokeExpr() }
65+
InvokeExpr getInvokeExpr() { result = this.(DataFlow::Impl::InvokeNodeDef).getInvokeExpr() }
6866

6967
/** Gets the name of the function or method being invoked, if it can be determined. */
70-
string getCalleeName() { result = impl.getCalleeName() }
68+
string getCalleeName() { result = this.(DataFlow::Impl::InvokeNodeDef).getCalleeName() }
7169

7270
/** Gets the data flow node specifying the function to be called. */
73-
DataFlow::Node getCalleeNode() { result = impl.getCalleeNode() }
71+
DataFlow::Node getCalleeNode() { result = this.(DataFlow::Impl::InvokeNodeDef).getCalleeNode() }
7472

7573
/**
7674
* Gets the data flow node corresponding to the `i`th argument of this invocation.
@@ -91,10 +89,10 @@ class InvokeNode extends DataFlow::SourceNode {
9189
* but the position of `z` cannot be determined, hence there are no first and second
9290
* argument nodes.
9391
*/
94-
DataFlow::Node getArgument(int i) { result = impl.getArgument(i) }
92+
DataFlow::Node getArgument(int i) { result = this.(DataFlow::Impl::InvokeNodeDef).getArgument(i) }
9593

9694
/** Gets the data flow node corresponding to an argument of this invocation. */
97-
DataFlow::Node getAnArgument() { result = impl.getAnArgument() }
95+
DataFlow::Node getAnArgument() { result = this.(DataFlow::Impl::InvokeNodeDef).getAnArgument() }
9896

9997
/** Gets the data flow node corresponding to the last argument of this invocation. */
10098
DataFlow::Node getLastArgument() { result = getArgument(getNumArgument() - 1) }
@@ -111,10 +109,12 @@ class InvokeNode extends DataFlow::SourceNode {
111109
* ```
112110
* .
113111
*/
114-
DataFlow::Node getASpreadArgument() { result = impl.getASpreadArgument() }
112+
DataFlow::Node getASpreadArgument() {
113+
result = this.(DataFlow::Impl::InvokeNodeDef).getASpreadArgument()
114+
}
115115

116116
/** Gets the number of arguments of this invocation, if it can be determined. */
117-
int getNumArgument() { result = impl.getNumArgument() }
117+
int getNumArgument() { result = this.(DataFlow::Impl::InvokeNodeDef).getNumArgument() }
118118

119119
Function getEnclosingFunction() { result = getBasicBlock().getContainer() }
120120

@@ -256,14 +256,14 @@ class InvokeNode extends DataFlow::SourceNode {
256256
* ```
257257
*/
258258
class CallNode extends InvokeNode {
259-
override DataFlow::Impl::CallNodeDef impl;
259+
CallNode() { this instanceof DataFlow::Impl::CallNodeDef }
260260

261261
/**
262262
* Gets the data flow node corresponding to the receiver expression of this method call.
263263
*
264264
* For example, the receiver of `x.m()` is `x`.
265265
*/
266-
DataFlow::Node getReceiver() { result = impl.getReceiver() }
266+
DataFlow::Node getReceiver() { result = this.(DataFlow::Impl::CallNodeDef).getReceiver() }
267267
}
268268

269269
/**
@@ -277,10 +277,10 @@ class CallNode extends InvokeNode {
277277
* ```
278278
*/
279279
class MethodCallNode extends CallNode {
280-
override DataFlow::Impl::MethodCallNodeDef impl;
280+
MethodCallNode() { this instanceof DataFlow::Impl::MethodCallNodeDef }
281281

282282
/** Gets the name of the invoked method, if it can be determined. */
283-
string getMethodName() { result = impl.getMethodName() }
283+
string getMethodName() { result = this.(DataFlow::Impl::MethodCallNodeDef).getMethodName() }
284284

285285
/**
286286
* Holds if this data flow node calls method `methodName` on receiver node `receiver`.
@@ -300,7 +300,7 @@ class MethodCallNode extends CallNode {
300300
* ```
301301
*/
302302
class NewNode extends InvokeNode {
303-
override DataFlow::Impl::NewNodeDef impl;
303+
NewNode() { this instanceof DataFlow::Impl::NewNodeDef }
304304
}
305305

306306
/**

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -268,9 +268,11 @@ module SocketIO {
268268

269269
/** Gets the `i`th parameter through which data is received from a client. */
270270
override DataFlow::SourceNode getReceivedItem(int i) {
271-
exists(DataFlow::FunctionNode cb | cb = getListener() and result = cb.getParameter(i) |
271+
exists(DataFlow::FunctionNode cb |
272+
cb = getListener() and
273+
result = cb.getParameter(i) and
272274
// exclude last parameter if it looks like a callback
273-
result != cb.getLastParameter() or not exists(result.getAnInvocation())
275+
not (result = cb.getLastParameter() and exists(result.getAnInvocation()))
274276
)
275277
}
276278

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

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,12 @@ private module PersistentWebStorage {
156156
result = DataFlow::globalVarRef(kind)
157157
}
158158

159+
pragma[noinline]
160+
WriteAccess getAWriteByName(string name, string kind) {
161+
result.getKey() = name and
162+
result.getKind() = kind
163+
}
164+
159165
/**
160166
* A read access.
161167
*/
@@ -165,8 +171,10 @@ private module PersistentWebStorage {
165171
ReadAccess() { this = webStorage(kind).getAMethodCall("getItem") }
166172

167173
override PersistentWriteAccess getAWrite() {
168-
getArgument(0).mayHaveStringValue(result.(WriteAccess).getKey()) and
169-
result.(WriteAccess).getKind() = kind
174+
exists(string name |
175+
getArgument(0).mayHaveStringValue(name) and
176+
result = getAWriteByName(name, kind)
177+
)
170178
}
171179
}
172180

0 commit comments

Comments
 (0)