Merge branch 'main' into post-release-prep/codeql-cli-2.10.1

javascript/ql/src/Declarations/UnusedVariable.ql

@@ -144,6 +144,9 @@ predicate whitelisted(UnusedLocal v) {
   // exclude variables mentioned in JSDoc comments in externs
   mentionedInJSDocComment(v)
   or
+  // the attributes in .vue files are not extracted, so we can get false positives in those.
+  v.getADeclaration().getFile().getExtension() = "vue"
+  or
   // exclude variables used to filter out unwanted properties
   isPropertyFilter(v)
   or

javascript/ql/src/Security/CWE-094/ImproperCodeSanitization.ql

@@ -50,7 +50,7 @@ private DataFlow::Node endsInCodeInjectionSink(DataFlow::TypeBackTracker t) {
     not result instanceof StringOps::ConcatenationRoot // the heuristic CodeInjection sink looks for string-concats, we are not interrested in those here.
   )
   or
-  exists(DataFlow::TypeBackTracker t2 | t = t2.smallstep(result, endsInCodeInjectionSink(t2)))
+  exists(DataFlow::TypeBackTracker t2 | t2 = t.smallstep(result, endsInCodeInjectionSink(t2)))
 }
 
 /**

javascript/ql/src/Security/CWE-178/CaseSensitiveMiddlewarePath.qhelp

@@ -0,0 +1,44 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>
+Using a case-sensitive regular expression path in a middleware route enables an attacker to bypass that middleware
+when accessing an endpoint with a case-insensitive path.
+Paths specified using a string are case-insensitive, whereas regular expressions are case-sensitive by default.
+</p>
+</overview>
+
+<recommendation>
+<p>
+When using a regular expression as a middleware path, make sure the regular expression is
+case-insensitive by adding the <code>i</code> flag.
+</p>
+</recommendation>
+
+<example>
+<p>
+The following example restricts access to paths in the <code>/admin</code> path to users logged in as
+administrators:
+</p>
+<sample src="examples/CaseSensitiveMiddlewarePath.js" />
+<p>
+A path such as <code>/admin/users/45</code> can only be accessed by an administrator. However, the path
+<code>/ADMIN/USERS/45</code> can be accessed by anyone because the upper-case path doesn't match the case-sensitive regular expression, whereas
+Express considers it to match the path string <code>/admin/users</code>.
+</p>
+<p>
+The issue can be fixed by adding the <code>i</code> flag to the regular expression:
+</p>
+<sample src="examples/CaseSensitiveMiddlewarePathGood.js" />
+</example>
+
+<references>
+<li>
+MDN
+<a href="https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_Expressions#advanced_searching_with_flags">Regular Expression Flags</a>.
+</li>
+</references>
+</qhelp>

javascript/ql/src/Security/CWE-178/CaseSensitiveMiddlewarePath.ql

@@ -0,0 +1,121 @@
+/**
+ * @name Case-sensitive middleware path
+ * @description Middleware with case-sensitive paths do not protect endpoints with case-insensitive paths.
+ * @kind problem
+ * @problem.severity warning
+ * @security-severity 7.3
+ * @precision high
+ * @id js/case-sensitive-middleware-path
+ * @tags security
+ *       external/cwe/cwe-178
+ */
+
+import javascript
+
+/**
+ * Converts `s` to upper case, or to lower-case if it was already upper case.
+ */
+bindingset[s]
+string toOtherCase(string s) {
+  if s.regexpMatch(".*[a-z].*") then result = s.toUpperCase() else result = s.toLowerCase()
+}
+
+RegExpCharacterClass getEnclosingClass(RegExpTerm term) {
+  term = result.getAChild()
+  or
+  term = result.getAChild().(RegExpRange).getAChild()
+}
+
+/**
+ * Holds if `term` seems to distinguish between upper and lower case letters, assuming the `i` flag is not present.
+ */
+pragma[inline]
+predicate isLikelyCaseSensitiveRegExp(RegExpTerm term) {
+  exists(RegExpConstant const |
+    const = term.getAChild*() and
+    const.getValue().regexpMatch(".*[a-zA-Z].*") and
+    not getEnclosingClass(const).getAChild().(RegExpConstant).getValue() =
+      toOtherCase(const.getValue()) and
+    not const.getParent*() instanceof RegExpNegativeLookahead and
+    not const.getParent*() instanceof RegExpNegativeLookbehind
+  )
+}
+
+/**
+ * Gets a string matched by `term`, or part of such a string.
+ */
+string getExampleString(RegExpTerm term) {
+  result = term.getAMatchedString()
+  or
+  // getAMatchedString does not recurse into sequences. Perform one step manually.
+  exists(RegExpSequence seq | seq = term |
+    result =
+      strictconcat(RegExpTerm child, int i, string text |
+        child = seq.getChild(i) and
+        (
+          text = child.getAMatchedString()
+          or
+          not exists(child.getAMatchedString()) and
+          text = ""
+        )
+      |
+        text order by i
+      )
+  )
+}
+
+string getCaseSensitiveBypassExample(RegExpTerm term) {
+  exists(string example |
+    example = getExampleString(term) and
+    result = toOtherCase(example) and
+    result != example // getting an example string is approximate; ensure we got a proper case-change example
+  )
+}
+
+/**
+ * Holds if `setup` has a path-argument `arg` referring to the given case-sensitive `regexp`.
+ */
+predicate isCaseSensitiveMiddleware(
+  Routing::RouteSetup setup, DataFlow::RegExpCreationNode regexp, DataFlow::Node arg
+) {
+  exists(DataFlow::MethodCallNode call |
+    setup = Routing::getRouteSetupNode(call) and
+    (
+      setup.definitelyResumesDispatch()
+      or
+      // If applied to all HTTP methods, be a bit more lenient in detecting middleware
+      setup.mayResumeDispatch() and
+      not exists(setup.getOwnHttpMethod())
+    ) and
+    arg = call.getArgument(0) and
+    regexp.getAReference().flowsTo(arg) and
+    isLikelyCaseSensitiveRegExp(regexp.getRoot()) and
+    exists(string flags |
+      flags = regexp.getFlags() and
+      not RegExp::isIgnoreCase(flags)
+    )
+  )
+}
+
+predicate isGuardedCaseInsensitiveEndpoint(
+  Routing::RouteSetup endpoint, Routing::RouteSetup middleware
+) {
+  isCaseSensitiveMiddleware(middleware, _, _) and
+  exists(DataFlow::MethodCallNode call |
+    endpoint = Routing::getRouteSetupNode(call) and
+    endpoint.isGuardedByNode(middleware) and
+    call.getArgument(0).mayHaveStringValue(_)
+  )
+}
+
+from
+  DataFlow::RegExpCreationNode regexp, Routing::RouteSetup middleware, Routing::RouteSetup endpoint,
+  DataFlow::Node arg, string example
+where
+  isCaseSensitiveMiddleware(middleware, regexp, arg) and
+  example = getCaseSensitiveBypassExample(regexp.getRoot()) and
+  isGuardedCaseInsensitiveEndpoint(endpoint, middleware) and
+  exists(endpoint.getRelativePath().toLowerCase().indexOf(example.toLowerCase()))
+select arg,
+  "This route uses a case-sensitive path $@, but is guarding a case-insensitive path $@. A path such as '"
+    + example + "' will bypass the middleware.", regexp, "pattern", endpoint, "here"

javascript/ql/src/Security/CWE-178/examples/CaseSensitiveMiddlewarePath.js

@@ -0,0 +1,13 @@
+const app = require('express')();
+
+app.use(/\/admin\/.*/, (req, res, next) => {
+    if (!req.user.isAdmin) {
+        res.status(401).send('Unauthorized');
+    } else {
+        next();
+    }
+});
+
+app.get('/admin/users/:id', (req, res) => {
+    res.send(app.database.users[req.params.id]);
+});

javascript/ql/src/Security/CWE-178/examples/CaseSensitiveMiddlewarePathGood.js

@@ -0,0 +1,13 @@
+const app = require('express')();
+
+app.use(/\/admin\/.*/i, (req, res, next) => {
+    if (!req.user.isAdmin) {
+        res.status(401).send('Unauthorized');
+    } else {
+        next();
+    }
+});
+
+app.get('/admin/users/:id', (req, res) => {
+    res.send(app.database.users[req.params.id]);
+});

javascript/ql/src/change-notes/2022-06-27-case-sensitive-middleware.md

@@ -0,0 +1,6 @@
+---
+category: newQuery
+---
+
+- A new query "Case-sensitive middleware path" (`js/case-sensitive-middleware-path`) has been added.
+  It highlights middleware routes that can be bypassed due to having a case-sensitive regular expression path.