From 5b26d03f1ca2dba1a91e86f363dd855e09529230 Mon Sep 17 00:00:00 2001 From: Erik Krogh Kristensen Date: Fri, 25 Oct 2019 15:45:35 +0200 Subject: [PATCH] introduce backtracking, and also marking join/slice calls --- ...atReturn.qhelp => IgnoreArrayResult.qhelp} | 16 ++++--- .../ql/src/Statements/IgnoreArrayResult.ql | 45 +++++++++++++++++++ .../ql/src/Statements/IgnoreConcatReturn.ql | 42 ----------------- .../{IgnoreConcat.js => IgnoreArrayResult.js} | 0 ...ncatFixed.js => IgnoreArrayResultFixed.js} | 0 .../IgnoreArrayResult.expected} | 0 .../IgnoreArrayResult.qlref} | 0 .../tst.js | 0 8 files changed, 55 insertions(+), 48 deletions(-) rename javascript/ql/src/Statements/{IgnoreConcatReturn.qhelp => IgnoreArrayResult.qhelp} (50%) create mode 100644 javascript/ql/src/Statements/IgnoreArrayResult.ql delete mode 100644 javascript/ql/src/Statements/IgnoreConcatReturn.ql rename javascript/ql/src/Statements/examples/{IgnoreConcat.js => IgnoreArrayResult.js} (100%) rename javascript/ql/src/Statements/examples/{IgnoreConcatFixed.js => IgnoreArrayResultFixed.js} (100%) rename javascript/ql/test/query-tests/Statements/{IgnoreConcatReturn/IgnoreConcatReturn.expected => IgnoreArrayResult/IgnoreArrayResult.expected} (100%) rename javascript/ql/test/query-tests/Statements/{IgnoreConcatReturn/IgnoreConcatReturn.qlref => IgnoreArrayResult/IgnoreArrayResult.qlref} (100%) rename javascript/ql/test/query-tests/Statements/{IgnoreConcatReturn => IgnoreArrayResult}/tst.js (100%) diff --git a/javascript/ql/src/Statements/IgnoreConcatReturn.qhelp b/javascript/ql/src/Statements/IgnoreArrayResult.qhelp similarity index 50% rename from javascript/ql/src/Statements/IgnoreConcatReturn.qhelp rename to javascript/ql/src/Statements/IgnoreArrayResult.qhelp index 1b69a61b3a9..b70ff1f1b32 100644 --- a/javascript/ql/src/Statements/IgnoreConcatReturn.qhelp +++ b/javascript/ql/src/Statements/IgnoreArrayResult.qhelp @@ -4,16 +4,18 @@

-The concat method on is pure and does not modify any of the input -arrays. It is therefore generally an error to ignore the return value from a -call to concat. +The concat, join and slice methods are +pure and does not modify any of the inputs or the array the method was called +on. It is therefore generally an error to ignore the return value from a call +to one of these methods.

-Use the returned value from the call to concat. +Use the returned value from the calls to concat, join +or slice.

@@ -26,19 +28,21 @@ function uses the concat method to add elements to the effect as the return value from concat is ignored.

- +

Assigning the returned value from the call to concat to the arr variable fixes the error.

- +
  • Mozilla Developer Network: Array concat.
    • +
  • Mozilla Developer Network: Array slice.
    • +
  • Mozilla Developer Network: Array join.
    •      diff --git a/javascript/ql/src/Statements/IgnoreArrayResult.ql b/javascript/ql/src/Statements/IgnoreArrayResult.ql new file mode 100644 index 00000000000..b3703c6992d --- /dev/null +++ b/javascript/ql/src/Statements/IgnoreArrayResult.ql @@ -0,0 +1,45 @@ +/** + * @name Ignoring result from pure array method + * @description The array methods do not modify the array, ignoring the result of such a call is therefore generally an error. + * @kind problem + * @problem.severity warning + * @id js/ignore-array-result + * @tags maintainability, + * correctness + * @precision high + */ + +import javascript +import Expressions.ExprHasNoEffect + +DataFlow::SourceNode callsArray(DataFlow::TypeBackTracker t, DataFlow::MethodCallNode call) { + isIgnoredPureArrayCall(call) and + ( + t.start() and + result = call.getReceiver() + or + exists(DataFlow::TypeBackTracker t2 | result = callsArray(t2, call).backtrack(t2, t)) + ) +} + +DataFlow::SourceNode callsArray(DataFlow::MethodCallNode call) { + result = callsArray(DataFlow::TypeBackTracker::end(), call) +} + +predicate isIgnoredPureArrayCall(DataFlow::MethodCallNode call) { + inVoidContext(call.asExpr()) and + ( + call.getMethodName() = "concat" and + call.getNumArgument() = 1 + or + call.getMethodName() = "join" and + call.getNumArgument() < 2 + or + call.getMethodName() = "slice" and + call.getNumArgument() < 3 + ) +} + +from DataFlow::MethodCallNode call +where callsArray(call) instanceof DataFlow::ArrayCreationNode +select call, "Result from call to " + call.getMethodName() + " ignored." diff --git a/javascript/ql/src/Statements/IgnoreConcatReturn.ql b/javascript/ql/src/Statements/IgnoreConcatReturn.ql deleted file mode 100644 index 65c379b3437..00000000000 --- a/javascript/ql/src/Statements/IgnoreConcatReturn.ql +++ /dev/null @@ -1,42 +0,0 @@ -/** - * @name Ignoring return from concat - * @description The concat method does not modify an array, ignoring the result of a call to concat is therefore generally an error. - * @kind problem - * @problem.severity warning - * @id js/ignore-result-from-concat - * @tags maintainability, - * correctness - * @precision high - */ - -import javascript -import Expressions.ExprHasNoEffect - -DataFlow::SourceNode array(DataFlow::TypeTracker t) { - t.start() and - result instanceof DataFlow::ArrayCreationNode - or - exists (DataFlow::TypeTracker t2 | - result = array(t2).track(t2, t) - ) -} - -DataFlow::SourceNode array() { result = array(DataFlow::TypeTracker::end()) } - -predicate isArrayMethod(DataFlow::MethodCallNode call) { - call.getReceiver().getALocalSource() = array() -} - -predicate isIncomplete(DataFlow::Node node) { - any(DataFlow::Incompleteness cause | node.analyze().getAValue().isIndefinite(cause)) != "global" -} - -from DataFlow::CallNode call -where - isArrayMethod(call) and - call.getCalleeName() = "concat" and - call.getNumArgument() = 1 and - (call.getArgument(0).getALocalSource() = array() or isIncomplete(call.getArgument(0))) and - not call.getArgument(0).asExpr().(ArrayExpr).getSize() = 0 and - inVoidContext(call.asExpr()) -select call, "Return value from call to concat ignored." diff --git a/javascript/ql/src/Statements/examples/IgnoreConcat.js b/javascript/ql/src/Statements/examples/IgnoreArrayResult.js similarity index 100% rename from javascript/ql/src/Statements/examples/IgnoreConcat.js rename to javascript/ql/src/Statements/examples/IgnoreArrayResult.js diff --git a/javascript/ql/src/Statements/examples/IgnoreConcatFixed.js b/javascript/ql/src/Statements/examples/IgnoreArrayResultFixed.js similarity index 100% rename from javascript/ql/src/Statements/examples/IgnoreConcatFixed.js rename to javascript/ql/src/Statements/examples/IgnoreArrayResultFixed.js diff --git a/javascript/ql/test/query-tests/Statements/IgnoreConcatReturn/IgnoreConcatReturn.expected b/javascript/ql/test/query-tests/Statements/IgnoreArrayResult/IgnoreArrayResult.expected similarity index 100% rename from javascript/ql/test/query-tests/Statements/IgnoreConcatReturn/IgnoreConcatReturn.expected rename to javascript/ql/test/query-tests/Statements/IgnoreArrayResult/IgnoreArrayResult.expected diff --git a/javascript/ql/test/query-tests/Statements/IgnoreConcatReturn/IgnoreConcatReturn.qlref b/javascript/ql/test/query-tests/Statements/IgnoreArrayResult/IgnoreArrayResult.qlref similarity index 100% rename from javascript/ql/test/query-tests/Statements/IgnoreConcatReturn/IgnoreConcatReturn.qlref rename to javascript/ql/test/query-tests/Statements/IgnoreArrayResult/IgnoreArrayResult.qlref diff --git a/javascript/ql/test/query-tests/Statements/IgnoreConcatReturn/tst.js b/javascript/ql/test/query-tests/Statements/IgnoreArrayResult/tst.js similarity index 100% rename from javascript/ql/test/query-tests/Statements/IgnoreConcatReturn/tst.js rename to javascript/ql/test/query-tests/Statements/IgnoreArrayResult/tst.js