From 5dcdf16a9626e63959bde982ea8d24af301769a5 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Thu, 22 Oct 2026 15:28:24 +0100 Subject: [PATCH 1/2] Fix multiline regular expression sanitizer guards Account for multiline and unknown flags in the shared whole-string anchoring check so generic and membership-based taint sanitizers agree. Add inline regression coverage for the reported command-injection and reflected-XSS cases, regex API variants, and class-validator decorators. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../2026-10-22-multiline-regexp-sanitizer.md | 4 + .../ql/lib/semmle/javascript/Regexp.qll | 28 +++-- .../RegExpSanitizer/decorators.ts | 15 +++ .../library-tests/RegExpSanitizer/guards.js | 112 ++++++++++++++++++ .../library-tests/RegExpSanitizer/routes.js | 36 ++++++ .../RegExpSanitizer/test.expected | 0 .../library-tests/RegExpSanitizer/test.ql | 40 +++++++ 7 files changed, 227 insertions(+), 8 deletions(-) create mode 100644 javascript/ql/lib/change-notes/2026-10-22-multiline-regexp-sanitizer.md create mode 100644 javascript/ql/test/library-tests/RegExpSanitizer/decorators.ts create mode 100644 javascript/ql/test/library-tests/RegExpSanitizer/guards.js create mode 100644 javascript/ql/test/library-tests/RegExpSanitizer/routes.js create mode 100644 javascript/ql/test/library-tests/RegExpSanitizer/test.expected create mode 100644 javascript/ql/test/library-tests/RegExpSanitizer/test.ql diff --git a/javascript/ql/lib/change-notes/2026-10-22-multiline-regexp-sanitizer.md b/javascript/ql/lib/change-notes/2026-10-22-multiline-regexp-sanitizer.md new file mode 100644 index 000000000000..d77bd99c1b0e --- /dev/null +++ b/javascript/ql/lib/change-notes/2026-10-22-multiline-regexp-sanitizer.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* Regular expression checks using multiline (`m`) or unknown flags are no longer treated as generic taint sanitizers just because the pattern is anchored with `^` and `$`. This can reveal previously missed alerts, including command injection and reflected cross-site scripting. diff --git a/javascript/ql/lib/semmle/javascript/Regexp.qll b/javascript/ql/lib/semmle/javascript/Regexp.qll index c42df3939c22..49702214bc62 100644 --- a/javascript/ql/lib/semmle/javascript/Regexp.qll +++ b/javascript/ql/lib/semmle/javascript/Regexp.qll @@ -1275,16 +1275,26 @@ module RegExp { bindingset[flags] predicate maybeDotAll(string flags) { flags = unknownFlag() or isDotAll(flags) } - /** Holds if `term` and all of its disjuncts are anchored on both ends. */ + /** + * Holds if `term` and all of its disjuncts are anchored to both ends of the input. + * + * Does not hold if a regular expression using the term has multiline or unknown flags. + */ predicate isFullyAnchoredTerm(RegExpTerm term) { - exists(RegExpSequence seq | term = seq | - seq.getChild(0) instanceof RegExpCaret and - seq.getLastChild() instanceof RegExpDollar + not exists(DataFlow::RegExpCreationNode regexp | + regexp.getRoot() = term.getRootTerm() and + maybeMultiline(regexp.tryGetFlags()) + ) and + ( + exists(RegExpSequence seq | term = seq | + seq.getChild(0) instanceof RegExpCaret and + seq.getLastChild() instanceof RegExpDollar + ) + or + isFullyAnchoredTerm(term.(RegExpGroup).getAChild()) + or + isFullyAnchoredAlt(term, term.getNumChild()) ) - or - isFullyAnchoredTerm(term.(RegExpGroup).getAChild()) - or - isFullyAnchoredAlt(term, term.getNumChild()) } /** Holds if the first `i` disjuncts of `term` are fully anchored. */ @@ -1330,6 +1340,8 @@ module RegExp { * * Specifically, whitelisting regexps such as `^(foo|bar)$` sanitize matches in the true case. * Inverted character classes such as `[^a-z]` or `\W` sanitize matches in the false case. + * Anchored terms do not sanitize matches if a regular expression using the term has + * multiline or unknown flags. */ predicate isGenericRegExpSanitizer(RegExpTerm term, boolean outcome) { term.isRootTerm() and diff --git a/javascript/ql/test/library-tests/RegExpSanitizer/decorators.ts b/javascript/ql/test/library-tests/RegExpSanitizer/decorators.ts new file mode 100644 index 000000000000..91a83e23d240 --- /dev/null +++ b/javascript/ql/test/library-tests/RegExpSanitizer/decorators.ts @@ -0,0 +1,15 @@ +import { Matches } from "class-validator"; + +class Validated { + @Matches(/^\w+$/) // $ sanitizer + singleLine: string; + + @Matches(/^\w+$/m) + multiline: string; + + @Matches(new RegExp("^\\w+$")) // $ sanitizer + constructedSingleLine: string; + + @Matches(new RegExp("^\\w+$", "m")) + constructedMultiline: string; +} diff --git a/javascript/ql/test/library-tests/RegExpSanitizer/guards.js b/javascript/ql/test/library-tests/RegExpSanitizer/guards.js new file mode 100644 index 000000000000..359329938585 --- /dev/null +++ b/javascript/ql/test/library-tests/RegExpSanitizer/guards.js @@ -0,0 +1,112 @@ +function literals() { + const value = source(); + sink(value); // $ flow + + if (/^[\w.-]+$/m.test(value)) { + sink(value); // $ flow + } + if (/^[\w.-]+$/.test(value)) { + sink(value); + } else { + sink(value); // $ flow + } + if (/^[\w.-]+$/i.test(value)) { + sink(value); + } + if (/^[\w.-]+$/s.test(value)) { + sink(value); + } + if (/^[\w.-]+$/m.exec(value)) { + sink(value); // $ flow + } + if (/^[\w.-]+$/.exec(value)) { + sink(value); + } + if (value.match(/^[\w.-]+$/m)) { + sink(value); // $ flow + } + if (value.match(/^[\w.-]+$/)) { + sink(value); + } + if (value.match("^[\\w.-]+$")) { + sink(value); + } + if (value.matchAll(/^[\w.-]+$/gm)) { + sink(value); // $ flow + } +} + +function references() { + const value = source(); + const regexp = /^[\w.-]+$/m; + if (regexp.test(value)) { + sink(value); // $ flow + } + let match; + if (match = regexp.exec(value)) { + sink(value); // $ flow + } +} + +function constructors(flags) { + const value = source(); + if (new RegExp("^[\\w.-]+$", "m").test(value)) { + sink(value); // $ flow + } + if (RegExp("^[\\w.-]+$", "im").test(value)) { + sink(value); // $ flow + } + const multiline = "m"; + if (new RegExp("^[\\w.-]+$", multiline).test(value)) { + sink(value); // $ flow + } + if (new RegExp("^[\\w.-]+$", flags).test(value)) { + sink(value); // $ flow + } + if (new RegExp("^[\\w.-]+$").test(value)) { + sink(value); + } + if (new RegExp("^[\\w.-]+$", "").test(value)) { + sink(value); + } + if (new RegExp("^[\\w.-]+$", "i").test(value)) { + sink(value); + } + if (new RegExp(/^[\w.-]+$/m).test(value)) { + sink(value); // $ flow + } + if (new RegExp(/^[\w.-]+$/, "m").test(value)) { + sink(value); // $ flow + } +} + +function groupsAndAlternatives() { + const value = source(); + if (/(^[\w.-]+$)/m.test(value)) { + sink(value); // $ flow + } + if (/(^[\w.-]+$)/.test(value)) { + sink(value); + } + if (/^foo$|^bar$/m.test(value)) { + sink(value); // $ flow + } + if (/^foo$|^bar$/.test(value)) { + sink(value); + } +} + +function negativeChecks(flags) { + const value = source(); + if (/[^a-z]/m.test(value)) { + sink(value); // $ flow + } else { + sink(value); + } + if (!/\W/m.test(value)) { + sink(value); + } + if (!new RegExp("[^a-z]", flags).test(value)) { + sink(value); + } +} diff --git a/javascript/ql/test/library-tests/RegExpSanitizer/routes.js b/javascript/ql/test/library-tests/RegExpSanitizer/routes.js new file mode 100644 index 000000000000..dabde8e13636 --- /dev/null +++ b/javascript/ql/test/library-tests/RegExpSanitizer/routes.js @@ -0,0 +1,36 @@ +const cp = require("child_process"); +const app = require("express")(); + +app.get("/a", (req, res) => { + const dir = req.query.dir; + if (/^[\w.-]+$/m.test(dir)) { + cp.exec("ls " + dir); // $ command-injection + } + res.send("ok"); +}); + +app.get("/b", (req, res) => { + const dir = req.query.dir; + if (/^[\w.-]+$/.test(dir)) { + cp.exec("ls " + dir); + } + res.send("ok"); +}); + +app.get("/c", (req, res) => { + cp.exec("ls " + req.query.dir); // $ command-injection + res.send("ok"); +}); + +app.get("/d", (req, res) => { + const name = req.query.name; + if (/^\w+$/m.test(name)) { + res.send("

Hello " + name + "

"); // $ reflected-xss + } else { + res.send("bad name"); + } +}); + +app.get("/e", (req, res) => { + res.send("

Hello " + req.query.name + "

"); // $ reflected-xss +}); diff --git a/javascript/ql/test/library-tests/RegExpSanitizer/test.expected b/javascript/ql/test/library-tests/RegExpSanitizer/test.expected new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/javascript/ql/test/library-tests/RegExpSanitizer/test.ql b/javascript/ql/test/library-tests/RegExpSanitizer/test.ql new file mode 100644 index 000000000000..107b616d81bd --- /dev/null +++ b/javascript/ql/test/library-tests/RegExpSanitizer/test.ql @@ -0,0 +1,40 @@ +import javascript +import utils.test.InlineExpectationsTest +import semmle.javascript.frameworks.ClassValidator +import semmle.javascript.security.dataflow.CommandInjectionQuery as CommandInjection +import semmle.javascript.security.dataflow.ReflectedXssQuery as ReflectedXss + +module TestConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { + source.(DataFlow::CallNode).getCalleeName() = "source" + } + + predicate isSink(DataFlow::Node sink) { + sink = any(DataFlow::CallNode call | call.getCalleeName() = "sink").getArgument(0) + } +} + +module TestFlow = TaintTracking::Global; + +module InlineTest implements TestSig { + string getARelevantTag() { result = ["flow", "command-injection", "reflected-xss", "sanitizer"] } + + predicate hasActualResult(Location location, string element, string tag, string value) { + element = "" and + value = "" and + exists(DataFlow::Node node | + location = node.getLocation() and + ( + tag = "flow" and TestFlow::flowTo(node) + or + tag = "command-injection" and CommandInjection::CommandInjectionFlow::flowTo(node) + or + tag = "reflected-xss" and ReflectedXss::ReflectedXssFlow::flowTo(node) + or + tag = "sanitizer" and node = ClassValidator::sanitizingDecorator() + ) + ) + } +} + +import MakeTest From a986a00b1ab7a1e17a9b138144316ec13af400dc Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Sat, 31 Oct 2026 00:44:40 +0000 Subject: [PATCH 2/2] Add tests documenting limitations of multiline detection --- .../test/library-tests/RegExpSanitizer/decorators.ts | 7 +++++++ .../ql/test/library-tests/RegExpSanitizer/guards.js | 11 +++++++++++ 2 files changed, 18 insertions(+) diff --git a/javascript/ql/test/library-tests/RegExpSanitizer/decorators.ts b/javascript/ql/test/library-tests/RegExpSanitizer/decorators.ts index 91a83e23d240..d987723ee7ea 100644 --- a/javascript/ql/test/library-tests/RegExpSanitizer/decorators.ts +++ b/javascript/ql/test/library-tests/RegExpSanitizer/decorators.ts @@ -12,4 +12,11 @@ class Validated { @Matches(new RegExp("^\\w+$", "m")) constructedMultiline: string; + + @Matches("^\\w+$", "i") // $ sanitizer + stringSingleLine: string; + + // Separate flags for string patterns are not currently covered by the sanitizer model. + @Matches("^\\w+$", "m") // $ SPURIOUS: sanitizer + stringMultiline: string; } diff --git a/javascript/ql/test/library-tests/RegExpSanitizer/guards.js b/javascript/ql/test/library-tests/RegExpSanitizer/guards.js index 359329938585..5428a1d0fe51 100644 --- a/javascript/ql/test/library-tests/RegExpSanitizer/guards.js +++ b/javascript/ql/test/library-tests/RegExpSanitizer/guards.js @@ -80,6 +80,17 @@ function constructors(flags) { } } +function overriddenFlags() { + const value = source(); + // Explicit overrides removing a literal's `m` flag are not currently covered by the sanitizer model. + if (new RegExp(/^[\w.-]+$/m, "").test(value)) { + sink(value); // $ SPURIOUS: flow + } + if (new RegExp(/^[\w.-]+$/m, "i").test(value)) { + sink(value); // $ SPURIOUS: flow + } +} + function groupsAndAlternatives() { const value = source(); if (/(^[\w.-]+$)/m.test(value)) {