From 2c43d1d731274586e215b3b6af7576660ae98793 Mon Sep 17 00:00:00 2001 From: Erik Krogh Kristensen Date: Mon, 23 Mar 2020 10:40:35 +0100 Subject: [PATCH 1/3] fix FP in superfluous-trailing-arguments related to Function.arguments --- javascript/ql/src/semmle/javascript/Functions.qll | 8 +++++++- .../LanguageFeatures/SpuriousArguments/tst.js | 10 ++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/javascript/ql/src/semmle/javascript/Functions.qll b/javascript/ql/src/semmle/javascript/Functions.qll index d6f0f965df7c..e5b3ec310c72 100644 --- a/javascript/ql/src/semmle/javascript/Functions.qll +++ b/javascript/ql/src/semmle/javascript/Functions.qll @@ -117,7 +117,13 @@ class Function extends @function, Parameterized, TypeParameterized, StmtContaine ArgumentsVariable getArgumentsVariable() { result.getFunction() = this } /** Holds if the body of this function refers to the function's `arguments` variable. */ - predicate usesArgumentsObject() { exists(getArgumentsVariable().getAnAccess()) } + predicate usesArgumentsObject() { + exists(getArgumentsVariable().getAnAccess()) or + exists(PropAccess read | + read.getBase() = getVariable().getAnAccess() and + read.getPropertyName() = "arguments" + ) + } /** * Holds if this function declares a parameter or local variable named `arguments`. diff --git a/javascript/ql/test/query-tests/LanguageFeatures/SpuriousArguments/tst.js b/javascript/ql/test/query-tests/LanguageFeatures/SpuriousArguments/tst.js index d521efa905fd..13877ff1ddac 100644 --- a/javascript/ql/test/query-tests/LanguageFeatures/SpuriousArguments/tst.js +++ b/javascript/ql/test/query-tests/LanguageFeatures/SpuriousArguments/tst.js @@ -120,3 +120,13 @@ parseFloat("123", 10); throwerWithParam(42, 87); // NOT OK throwerIndirect(42); // OK, but still flagged due to complexity }); + +function sum2() { + var result = 0; + for (var i=0,n=sum2.arguments.length; i Date: Mon, 23 Mar 2020 14:10:07 +0100 Subject: [PATCH 2/3] autoformat --- javascript/ql/src/semmle/javascript/Functions.qll | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/javascript/ql/src/semmle/javascript/Functions.qll b/javascript/ql/src/semmle/javascript/Functions.qll index e5b3ec310c72..719e094dfc02 100644 --- a/javascript/ql/src/semmle/javascript/Functions.qll +++ b/javascript/ql/src/semmle/javascript/Functions.qll @@ -118,8 +118,9 @@ class Function extends @function, Parameterized, TypeParameterized, StmtContaine /** Holds if the body of this function refers to the function's `arguments` variable. */ predicate usesArgumentsObject() { - exists(getArgumentsVariable().getAnAccess()) or - exists(PropAccess read | + exists(getArgumentsVariable().getAnAccess()) + or + exists(PropAccess read | read.getBase() = getVariable().getAnAccess() and read.getPropertyName() = "arguments" ) From 833183c706300a7f92c3d0758234824376e028c0 Mon Sep 17 00:00:00 2001 From: Erik Krogh Kristensen Date: Mon, 23 Mar 2020 14:13:30 +0100 Subject: [PATCH 3/3] change note --- change-notes/1.24/analysis-javascript.md | 1 + 1 file changed, 1 insertion(+) diff --git a/change-notes/1.24/analysis-javascript.md b/change-notes/1.24/analysis-javascript.md index b3241de5adb0..00f33bcacc23 100644 --- a/change-notes/1.24/analysis-javascript.md +++ b/change-notes/1.24/analysis-javascript.md @@ -80,6 +80,7 @@ | Use of password hash with insufficient computational effort (`js/insufficient-password-hash`) | Fewer false positive results | This query now recognizes additional cases that do not require secure hashing. | | Useless regular-expression character escape (`js/useless-regexp-character-escape`) | Fewer false positive results | This query now distinguishes escapes in strings and regular expression literals. | | Identical operands (`js/redundant-operation`) | Fewer results | This query now recognizes cases where the operands change a value using ++/-- expressions. | +| Superfluous trailing arguments (`js/superfluous-trailing-arguments`) | Fewer results | This query now recognizes cases where a function uses the `Function.arguments` value to process a variable number of parameters. | ## Changes to libraries