hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2025-12-17 09:13:20 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #18547 from erik-krogh/suffixCheck

Browse Source 
JS: Fix FPs with js/incorrect-suffix-check
This commit is contained in:
Erik Krogh Kristensen
2025-01-22 21:13:27 +01:00
committed by GitHub
parent 546a4971d5 04bbd5919a
commit 4bd4937e65
3 changed files with 29 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
javascript/ql/src/Security/CWE-020/IncorrectSuffixCheck.ql
View File

@@ -44,12 +44,25 @@ class IndexOfCall extends DataFlow::MethodCallNode {
* Gets an `indexOf` call with the same receiver, argument, and method name, including this call itself. * Gets an `indexOf` call with the same receiver, argument, and method name, including this call itself.
*/ */
IndexOfCall getAnEquivalentIndexOfCall() { IndexOfCall getAnEquivalentIndexOfCall() {
result = this
or
exists(DataFlow::Node recv, string m | exists(DataFlow::Node recv, string m |
this.receiverAndMethodName(recv, m) and result.receiverAndMethodName(recv, m) this.receiverAndMethodName(recv, m) and result.receiverAndMethodName(recv, m)
| |
// both directly reference the same value
result.getArgument(0).getALocalSource() = this.getArgument(0).getALocalSource() result.getArgument(0).getALocalSource() = this.getArgument(0).getALocalSource()
or or
// both use the same string literal
result.getArgument(0).getStringValue() = this.getArgument(0).getStringValue() result.getArgument(0).getStringValue() = this.getArgument(0).getStringValue()
or
// both use the same concatenation of a string and a value
exists(Expr origin, StringLiteral str, AddExpr otherAdd |
this.getArgument(0).asExpr().(AddExpr).hasOperands(origin, str) and
otherAdd = result.getArgument(0).asExpr()
|
otherAdd.getAnOperand().(StringLiteral).getStringValue() = str.getStringValue() and
otherAdd.getAnOperand().flow().getALocalSource() = origin.flow().getALocalSource()
)
) )
} }

4
javascript/ql/src/change-notes/2025-01-22-indexof-suffix-check.md Normal file
View File

@@ -0,0 +1,4 @@
---
category: majorAnalysis
---
* The `js/incorrect-suffix-check` query now recognises some good patterns of the form `origin.indexOf("." + allowedOrigin)` that were previously falsely flagged.

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

@@ -97,3 +97,15 @@ function lastIndexNeqMinusOne(x) {
function lastIndexEqMinusOne(x) { function lastIndexEqMinusOne(x) {
return x.lastIndexOf("example.com") === -1 || x.lastIndexOf("example.com") === x.length - "example.com".length; // OK return x.lastIndexOf("example.com") === -1 || x.lastIndexOf("example.com") === x.length - "example.com".length; // OK
} }
function sameCheck(allowedOrigin) {
const trustedAuthority = "example.com";
const ind = trustedAuthority.indexOf("." + allowedOrigin);
return ind > 0 && ind === trustedAuthority.length - allowedOrigin.length - 1; // OK
}
function sameConcatenation(allowedOrigin) {
const trustedAuthority = "example.com";
return trustedAuthority.indexOf("." + allowedOrigin) > 0 && trustedAuthority.indexOf("." + allowedOrigin) === trustedAuthority.length - allowedOrigin.length - 1; // OK
}