Skip to content

Commit 4bd4937

Browse files
authored
Merge pull request github#18547 from erik-krogh/suffixCheck
JS: Fix FPs with js/incorrect-suffix-check
2 parents 546a497 + 04bbd59 commit 4bd4937

File tree

3 files changed

+29
-0
lines changed

3 files changed

+29
-0
lines changed

javascript/ql/src/Security/CWE-020/IncorrectSuffixCheck.ql

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,12 +44,25 @@ class IndexOfCall extends DataFlow::MethodCallNode {
4444
* Gets an `indexOf` call with the same receiver, argument, and method name, including this call itself.
4545
*/
4646
IndexOfCall getAnEquivalentIndexOfCall() {
47+
result = this
48+
or
4749
exists(DataFlow::Node recv, string m |
4850
this.receiverAndMethodName(recv, m) and result.receiverAndMethodName(recv, m)
4951
|
52+
// both directly reference the same value
5053
result.getArgument(0).getALocalSource() = this.getArgument(0).getALocalSource()
5154
or
55+
// both use the same string literal
5256
result.getArgument(0).getStringValue() = this.getArgument(0).getStringValue()
57+
or
58+
// both use the same concatenation of a string and a value
59+
exists(Expr origin, StringLiteral str, AddExpr otherAdd |
60+
this.getArgument(0).asExpr().(AddExpr).hasOperands(origin, str) and
61+
otherAdd = result.getArgument(0).asExpr()
62+
|
63+
otherAdd.getAnOperand().(StringLiteral).getStringValue() = str.getStringValue() and
64+
otherAdd.getAnOperand().flow().getALocalSource() = origin.flow().getALocalSource()
65+
)
5366
)
5467
}
5568

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: majorAnalysis
3+
---
4+
* The `js/incorrect-suffix-check` query now recognises some good patterns of the form `origin.indexOf("." + allowedOrigin)` that were previously falsely flagged.

javascript/ql/test/query-tests/Security/CWE-020/IncorrectSuffixCheck/tst.js

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,3 +97,15 @@ function lastIndexNeqMinusOne(x) {
9797
function lastIndexEqMinusOne(x) {
9898
return x.lastIndexOf("example.com") === -1 || x.lastIndexOf("example.com") === x.length - "example.com".length; // OK
9999
}
100+
101+
function sameCheck(allowedOrigin) {
102+
const trustedAuthority = "example.com";
103+
104+
const ind = trustedAuthority.indexOf("." + allowedOrigin);
105+
return ind > 0 && ind === trustedAuthority.length - allowedOrigin.length - 1; // OK
106+
}
107+
108+
function sameConcatenation(allowedOrigin) {
109+
const trustedAuthority = "example.com";
110+
return trustedAuthority.indexOf("." + allowedOrigin) > 0 && trustedAuthority.indexOf("." + allowedOrigin) === trustedAuthority.length - allowedOrigin.length - 1; // OK
111+
}

0 commit comments

Comments
 (0)