From 15d74b7d032cfa0e922d769d0a342eb8e6a5f8a6 Mon Sep 17 00:00:00 2001 From: Erik Krogh Kristensen Date: Mon, 16 Dec 2019 16:36:58 +0100 Subject: [PATCH] remove FP from js/regexpinjection where no regexp was constructed --- javascript/ql/src/semmle/javascript/Regexp.qll | 3 ++- .../test/query-tests/Security/CWE-730/RegExpInjection.js | 9 +++++++++ .../ql/test/query-tests/Security/CWE-730/search.js | 6 ++++++ 3 files changed, 17 insertions(+), 1 deletion(-) create mode 100644 javascript/ql/test/query-tests/Security/CWE-730/search.js diff --git a/javascript/ql/src/semmle/javascript/Regexp.qll b/javascript/ql/src/semmle/javascript/Regexp.qll index f0207da63eb..07060d04742 100644 --- a/javascript/ql/src/semmle/javascript/Regexp.qll +++ b/javascript/ql/src/semmle/javascript/Regexp.qll @@ -810,7 +810,8 @@ predicate isInterpretedAsRegExp(DataFlow::Node source) { // The argument of a call that coerces the argument to a regular expression. exists(MethodCallExpr mce, string methodName | mce.getReceiver().analyze().getAType() = TTString() and - mce.getMethodName() = methodName + mce.getMethodName() = methodName and + not exists(DataFlow::FunctionNode func | func = DataFlow::valueNode(mce.getCallee()).getAFunctionValue() | not func.getFunction().inExternsFile()) | methodName = "match" and source.asExpr() = mce.getArgument(0) and mce.getNumArgument() = 1 or diff --git a/javascript/ql/test/query-tests/Security/CWE-730/RegExpInjection.js b/javascript/ql/test/query-tests/Security/CWE-730/RegExpInjection.js index 5d1288f3a7e..2db6339c7ba 100644 --- a/javascript/ql/test/query-tests/Security/CWE-730/RegExpInjection.js +++ b/javascript/ql/test/query-tests/Security/CWE-730/RegExpInjection.js @@ -50,4 +50,13 @@ app.get('/findKey', function(req, res) { URI(`${protocol}://${host}${path}`).search(input); // OK, but still flagged URI(`${protocol}://${host}${path}`).search(input).href(); // OK unknown.search(input).unknown; // OK + +}); + +import * as Search from './search'; + +app.get('/findKey', function(req, res) { + var key = req.param("key"), input = req.param("input"); + + Search.search(input); // OK! }); diff --git a/javascript/ql/test/query-tests/Security/CWE-730/search.js b/javascript/ql/test/query-tests/Security/CWE-730/search.js new file mode 100644 index 00000000000..60fa6ea2f0e --- /dev/null +++ b/javascript/ql/test/query-tests/Security/CWE-730/search.js @@ -0,0 +1,6 @@ +module.someOtherExport = true; + + +export function search(query) { + // Do nothing! +}