Skip to content

Commit 2423c77

Browse files
erik-krogherik-krogh
authored
Merge pull request #9281 from erik-krogh/jsQL
JS: various QL-for-QL fixes
2 parents 07e450d + b2d3a7d commit 2423c77

File tree

14 files changed

+100
-91
lines changed

14 files changed

+100
-91
lines changed

‎javascript/ql/lib/change-notes/2022-05-24-isLibraryFile.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* The `isLibaryFile` predicate from `ClassifyFiles.qll` has been renamed to `isLibraryFile` to fix a typo.

‎javascript/ql/lib/semmle/javascript/dataflow/TaintTracking.qll

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ module TaintTracking {
8787
override predicate isLabeledBarrier(DataFlow::Node node, DataFlow::FlowLabel lbl) {
8888
super.isLabeledBarrier(node, lbl)
8989
or
90-
isSanitizer(node) and lbl.isTaint()
90+
this.isSanitizer(node) and lbl.isTaint()
9191
}
9292

9393
override predicate isBarrier(DataFlow::Node node) {
@@ -103,15 +103,15 @@ module TaintTracking {
103103
) {
104104
super.isBarrierEdge(source, sink, lbl)
105105
or
106-
isSanitizerEdge(source, sink, lbl)
106+
this.isSanitizerEdge(source, sink, lbl)
107107
or
108-
isSanitizerEdge(source, sink) and lbl.isTaint()
108+
this.isSanitizerEdge(source, sink) and lbl.isTaint()
109109
}
110110

111111
final override predicate isBarrierGuard(DataFlow::BarrierGuardNode guard) {
112112
super.isBarrierGuard(guard) or
113113
guard.(AdditionalSanitizerGuardNode).appliesTo(this) or
114-
isSanitizerGuard(guard)
114+
this.isSanitizerGuard(guard)
115115
}
116116

117117
/**
@@ -121,14 +121,14 @@ module TaintTracking {
121121
predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) { none() }
122122

123123
final override predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) {
124-
isAdditionalTaintStep(pred, succ) or
124+
this.isAdditionalTaintStep(pred, succ) or
125125
sharedTaintStep(pred, succ)
126126
}
127127

128128
final override predicate isAdditionalFlowStep(
129129
DataFlow::Node pred, DataFlow::Node succ, boolean valuePreserving
130130
) {
131-
isAdditionalFlowStep(pred, succ) and valuePreserving = false
131+
this.isAdditionalFlowStep(pred, succ) and valuePreserving = false
132132
}
133133

134134
override DataFlow::FlowLabel getDefaultSourceLabel() { result.isTaint() }
@@ -173,9 +173,9 @@ module TaintTracking {
173173
abstract predicate sanitizes(boolean outcome, Expr e);
174174

175175
override predicate blocks(boolean outcome, Expr e, DataFlow::FlowLabel label) {
176-
sanitizes(outcome, e) and label.isTaint()
176+
this.sanitizes(outcome, e) and label.isTaint()
177177
or
178-
sanitizes(outcome, e, label)
178+
this.sanitizes(outcome, e, label)
179179
}
180180

181181
/**
@@ -1032,13 +1032,13 @@ module TaintTracking {
10321032
name = "has" or
10331033
name = "hasOwnProperty"
10341034
|
1035-
getMethodName() = name
1035+
this.getMethodName() = name
10361036
)
10371037
}
10381038

10391039
override predicate sanitizes(boolean outcome, Expr e) {
10401040
outcome = true and
1041-
e = getArgument(0).asExpr()
1041+
e = this.getArgument(0).asExpr()
10421042
}
10431043

10441044
override predicate appliesTo(Configuration cfg) { any() }
@@ -1053,14 +1053,14 @@ module TaintTracking {
10531053
*/
10541054
class AdHocWhitelistCheckSanitizer extends SanitizerGuardNode, DataFlow::CallNode {
10551055
AdHocWhitelistCheckSanitizer() {
1056-
getCalleeName()
1056+
this.getCalleeName()
10571057
.regexpMatch("(?i).*((?<!un)safe|whitelist|(?<!in)valid|allow|(?<!un)auth(?!or\\b)).*") and
1058-
getNumArgument() = 1
1058+
this.getNumArgument() = 1
10591059
}
10601060

10611061
override predicate sanitizes(boolean outcome, Expr e) {
10621062
outcome = true and
1063-
e = getArgument(0).asExpr()
1063+
e = this.getArgument(0).asExpr()
10641064
}
10651065
}
10661066

‎javascript/ql/lib/semmle/javascript/filters/ClassifyFiles.qll

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,10 @@ predicate isExternsFile(File f) {
7373
/**
7474
* Holds if `f` contains library code.
7575
*/
76-
predicate isLibaryFile(File f) { f.getATopLevel() instanceof FrameworkLibraryInstance }
76+
predicate isLibraryFile(File f) { f.getATopLevel() instanceof FrameworkLibraryInstance }
77+
78+
/** DEPRECATED: Alias for isLibraryFile */
79+
deprecated predicate isLibaryFile = isLibraryFile/1;
7780

7881
/**
7982
* Holds if `f` contains template code.
@@ -106,7 +109,7 @@ predicate classify(File f, string category) {
106109
or
107110
isExternsFile(f) and category = "externs"
108111
or
109-
isLibaryFile(f) and category = "library"
112+
isLibraryFile(f) and category = "library"
110113
or
111114
isTemplateFile(f) and category = "template"
112115
}

‎javascript/ql/lib/semmle/javascript/frameworks/ClosureLibrary.qll

Lines changed: 32 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -13,38 +13,40 @@ module ClosureLibrary {
1313
call = Closure::moduleImport("goog.string." + name).getACall() and succ = call
1414
|
1515
pred = call.getAnArgument() and
16-
(
17-
name = "canonicalizeNewlines" or
18-
name = "capitalize" or
19-
name = "collapseBreakingSpaces" or
20-
name = "collapseWhitespace" or
21-
name = "format" or
22-
name = "makeSafe" or // makeSafe just guards against null and undefined
23-
name = "newLineOrBr" or
24-
name = "normalizeSpaces" or
25-
name = "normalizeWhitespace" or
26-
name = "preserveSpaces" or
27-
name = "remove" or // removes first occurrence of a substring
28-
name = "repeat" or
29-
name = "splitLimit" or
30-
name = "stripNewlines" or
31-
name = "subs" or
32-
name = "toCamelCase" or
33-
name = "toSelectorCase" or
34-
name = "toTitleCase" or
35-
name = "trim" or
36-
name = "trimLeft" or
37-
name = "trimRight" or
38-
name = "unescapeEntities" or
39-
name = "whitespaceEscape"
40-
)
16+
name =
17+
[
18+
"canonicalizeNewlines", //
19+
"capitalize", //
20+
"collapseBreakingSpaces", //
21+
"collapseWhitespace", //
22+
"format", //
23+
"makeSafe", // makeSafe just guards against null and undefined
24+
"newLineOrBr", //
25+
"normalizeSpaces", //
26+
"normalizeWhitespace", //
27+
"preserveSpaces", //
28+
"remove", // removes first occurrence of a substring
29+
"repeat", //
30+
"splitLimit", //
31+
"stripNewlines", //
32+
"subs", //
33+
"toCamelCase", //
34+
"toSelectorCase", //
35+
"toTitleCase", //
36+
"trim", //
37+
"trimLeft", //
38+
"trimRight", //
39+
"unescapeEntities", //
40+
"whitespaceEscape"
41+
]
4142
or
4243
pred = call.getArgument(0) and
43-
(
44-
name = "truncate" or
45-
name = "truncateMiddle" or
46-
name = "unescapeEntitiesWithDocument"
47-
)
44+
name =
45+
[
46+
"truncate", //
47+
"truncateMiddle", //
48+
"unescapeEntitiesWithDocument", //
49+
]
4850
)
4951
}
5052
}

‎javascript/ql/lib/semmle/javascript/frameworks/Handlebars.qll

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -150,8 +150,6 @@ private module HandlebarsTaintSteps {
150150
* helpers registered.
151151
*/
152152
class HandlebarsStep extends DataFlow::SharedFlowStep {
153-
DataFlow::CallNode templatingCall;
154-
155153
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
156154
isHandlebarsArgStep(pred, succ)
157155
}

‎javascript/ql/lib/semmle/javascript/frameworks/History.qll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,10 @@ module History {
3535
/**
3636
* A user-controlled location value read from the [history](http://npmjs.org/package/history) library.
3737
*/
38-
private class HistoryLibaryRemoteFlow extends ClientSideRemoteFlowSource {
38+
private class HistoryLibraryRemoteFlow extends ClientSideRemoteFlowSource {
3939
ClientSideRemoteFlowKind kind;
4040

41-
HistoryLibaryRemoteFlow() {
41+
HistoryLibraryRemoteFlow() {
4242
exists(API::Node loc | loc = [getBrowserHistory(), getHashHistory()].getMember("location") |
4343
this = loc.getMember("hash").getAnImmediateUse() and kind.isFragment()
4444
or

‎javascript/ql/lib/semmle/javascript/frameworks/UriLibraries.qll

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -362,29 +362,31 @@ private module ClosureLibraryUri {
362362
// static methods in goog.uri.utils
363363
arg = 0 and
364364
exists(string name | invoke = Closure::moduleImport("goog.uri.utils." + name).getACall() |
365-
name = "appendParam" or // preserve taint from the original URI, but not from the appended param
366-
name = "appendParams" or
367-
name = "appendParamsFromMap" or
368-
name = "appendPath" or
369-
name = "getParamValue" or
370-
name = "getParamValues" or
371-
name = "getPath" or
372-
name = "getPathAndAfter" or
373-
name = "getQueryData" or
374-
name = "parseQueryData" or
375-
name = "removeFragment" or
376-
name = "removeParam" or
377-
name = "setParam" or
378-
name = "setParamsFromMap" or
379-
name = "setPath" or
380-
name = "split"
365+
name =
366+
[
367+
"appendParam", // preserve taint from the original URI, but not from the appended param
368+
"appendParams", //
369+
"appendParamsFromMap", //
370+
"appendPath", //
371+
"getParamValue", //
372+
"getParamValues", //
373+
"getPath", //
374+
"getPathAndAfter", //
375+
"getQueryData", //
376+
"parseQueryData", //
377+
"removeFragment", //
378+
"removeParam", //
379+
"setParam", //
380+
"setParamsFromMap", //
381+
"setPath", //
382+
"split", //
383+
]
381384
)
382385
or
383386
// static methods in goog.string
384387
arg = 0 and
385388
exists(string name | invoke = Closure::moduleImport("goog.string." + name).getACall() |
386-
name = "urlDecode" or
387-
name = "urlEncode"
389+
name = ["urlDecode", "urlEncode"]
388390
)
389391
)
390392
}

‎javascript/ql/lib/semmle/javascript/security/dataflow/ExceptionXssCustomizations.qll

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
*/
66

77
import javascript
8-
import semmle.javascript.security.dataflow.RemoteFlowSources
98

109
/**
1110
* Provides sources, sinks, and sanitizers for reasoning about

‎javascript/ql/src/Expressions/StringInsteadOfRegex.ql

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,16 @@ import javascript
1414
* Gets a regular expression pattern that matches the syntax of likely regular expressions.
1515
*/
1616
private string getALikelyRegExpPattern() {
17-
result = "/.*/[gimuy]{1,5}" or // pattern with at least one flag: /foo/i
18-
result = "/\\^.*/[gimuy]{0,5}" or // pattern with anchor: /^foo/
19-
result = "/.*\\$/[gimuy]{0,5}" or // pattern with anchor: /foo$/
20-
result = "\\^.*\\$" or // pattern body with anchors: ^foo$
21-
result = ".*(?<!\\\\)\\\\[dDwWsSB].*" or // contains a builtin character class: \s
22-
result = ".*(?<!\\\\)\\\\[\\[\\]()*+?{}|^$.].*" or // contains an escaped meta-character: \(
23-
result = ".*\\[\\^?[\\p{Alnum}\\p{Blank}_-]+\\][*+].*" // contains a quantified custom character class: [^a-zA-Z123]+
17+
result =
18+
[
19+
"/.*/[gimuy]{1,5}", // pattern with at least one flag: /foo/i
20+
"/\\^.*/[gimuy]{0,5}", // pattern with anchor: /^foo/
21+
"/.*\\$/[gimuy]{0,5}", // pattern with anchor: /foo$/
22+
"\\^.*\\$", // pattern body with anchors: ^foo$
23+
".*(?<!\\\\)\\\\[dDwWsSB].*", // contains a builtin character class: \s
24+
".*(?<!\\\\)\\\\[\\[\\]()*+?{}|^$.].*", // contains an escaped meta-character: \(
25+
".*\\[\\^?[\\p{Alnum}\\p{Blank}_-]+\\][*+].*" // contains a quantified custom character class: [^a-zA-Z123]+
26+
]
2427
}
2528

2629
/**

‎javascript/ql/src/Security/CWE-020/IncompleteUrlSchemeCheck.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ class DangerousScheme extends string {
2424
string getWithoutColon() { this = result + ":" }
2525

2626
/** Gets the name of this scheme, with or without the `:`. */
27-
string getWithOrWithoutColon() { result = this or result = getWithoutColon() }
27+
string getWithOrWithoutColon() { result = this or result = this.getWithoutColon() }
2828
}
2929

3030
/** Returns a node that refers to the scheme of `url`. */

0 commit comments

Comments
 (0)