Javascript: Regex Global Flag in Test Function

javascript/ql/src/experimental/Security/CWE-020/RegexGlobalFlagBad.ts

@@ -0,0 +1,19 @@
+import { Request, Response, Application } from 'express';
+import express from 'express';
+import db from './postgres';
+
+const FORBIDDEN_CHARS = /['\\]/g;
+const isForbidden = (str: string) => FORBIDDEN_CHARS.test(str);
+
+const app: Application = express();
+
+app.get('/api/users/:name', async (req: Request, res: Response) => {
+    const { name } = req.params;
+    if (isForbidden(name)) {
+        return res.sendStatus(400);
+    }
+    const user = await db.query(`SELECT * FROM users WHERE name='${name}'`);
+    res.json(user);
+});
+
+app.listen(1337);

javascript/ql/src/experimental/Security/CWE-020/RegexGlobalFlagGood.ts

@@ -0,0 +1,19 @@
+import { Request, Response, Application } from 'express';
+import express from 'express';
+import db from './postgres';
+
+const FORBIDDEN_CHARS = /['\\]/;
+const isForbidden = (str: string) => FORBIDDEN_CHARS.test(str);
+
+const app: Application = express();
+
+app.get('/api/users/:name', async (req: Request, res: Response) => {
+    const { name } = req.params;
+    if (isForbidden(name)) {
+        return res.sendStatus(400);
+    }
+    const user = await db.query(`SELECT * FROM users WHERE name='${name}'`);
+    res.json(user);
+});
+
+app.listen(1337);

javascript/ql/src/experimental/Security/CWE-020/RegexValidation.qhelp

@@ -0,0 +1,33 @@
+<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
+<qhelp>
+    <overview>
+        <p>
+            The use of the global flag in regular expressions in JavaScript can lead to unexpected behaviors in the test function. This issue arises because the global regex maintains its last index position across multiple calls, resulting in the test function sometimes returning true and other times false for the same input.
+        </p>
+    </overview>
+    <recommendation>
+        <p>
+            To avoid this issue, it is recommended to either avoid using the global flag with the test method or reset the lastIndex of the regular expression to 0 before each test call. Alternatively, use the match method for scenarios where global search is required.
+        </p>
+    </recommendation>
+    <example>
+        <p>
+            Vulnerable code example: Using a global regex in a test function without resetting lastIndex. The function may return inconsistent results over repeated calls.
+        </p>
+        <sample src="RegexGlobalFlagBad.ts" />
+    </example>
+    <example>
+        <p>
+            Secure code example: Resetting the lastIndex of the regex to 0 before each test call or using match for global searches.
+        </p>
+        <sample src="RegexGlobalFlagGood.ts" />
+    </example>
+    <references>
+        <li>
+            MDN Web Docs - Regular Expressions:
+            <a href="https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_Expressions">Understanding Regular Expressions in JavaScript</a>
+            <a href="https://stackoverflow.com/questions/28910934/why-does-regex-test-changes-the-result-in-subsequent-calls">Example Stackoverflow Discussion</a>
+        </li>
+        <!-- Additional references can be added here -->
+    </references>
+</qhelp>

javascript/ql/src/experimental/Security/CWE-020/RegexValidation.ql

@@ -0,0 +1,27 @@
+/**
+ * @name Regex Global Flag in Test Function
+ * @description When using the global flag (g) with regex in JavaScript, the test function
+ *              may return inconsistent results (true or false) across multiple calls.
+ *              This is due to the regex maintaining its last index position between calls.
+ *              This behavior can lead to unexpected bugs, especially in validation scenarios.
+ * @kind problem
+ * @problem.severity error
+ * @security-severity 6
+ * @precision medium
+ * @id js/regex-global-flag-issue
+ * @tags javascript
+ *       regex
+ *       cwe-020
+ *       global-flag
+ *       testing
+ *       bug
+ */
+
+import javascript
+
+from RegExpLiteral re, CallExpr call, VariableAccess va
+where
+  re.getFlags().regexpMatch("g") and
+  call.getCalleeName() = "test" and
+  call.getArgument(0) = va
+select re, "This call to " + call + " uses a regular expression with a global flag."

javascript/ql/test/experimental/Security/CWE-020/RegexGlobalFlagBad.ts

@@ -0,0 +1,19 @@
+import { Request, Response, Application } from 'express';
+import express from 'express';
+import db from './postgres';
+
+const FORBIDDEN_CHARS = /['\\]/g;
+const isForbidden = (str: string) => FORBIDDEN_CHARS.test(str);
+
+const app: Application = express();
+
+app.get('/api/users/:name', async (req: Request, res: Response) => {
+    const { name } = req.params;
+    if (isForbidden(name)) {
+        return res.sendStatus(400);
+    }
+    const user = await db.query(`SELECT * FROM users WHERE name='${name}'`);
+    res.json(user);
+});
+
+app.listen(1337);

javascript/ql/test/experimental/Security/CWE-020/RegexGlobalFlagGood.ts

@@ -0,0 +1,19 @@
+import { Request, Response, Application } from 'express';
+import express from 'express';
+import db from './postgres';
+
+const FORBIDDEN_CHARS = /['\\]/;
+const isForbidden = (str: string) => FORBIDDEN_CHARS.test(str);
+
+const app: Application = express();
+
+app.get('/api/users/:name', async (req: Request, res: Response) => {
+    const { name } = req.params;
+    if (isForbidden(name)) {
+        return res.sendStatus(400);
+    }
+    const user = await db.query(`SELECT * FROM users WHERE name='${name}'`);
+    res.json(user);
+});
+
+app.listen(1337);

javascript/ql/test/experimental/Security/CWE-020/RegexValidation.expected

@@ -0,0 +1 @@
+| RegexGlobalFlagBad.ts:5:25:5:32 | /['\\\\]/g | This call to FORBIDD ... st(str) uses a regular expression with a global flag. |

javascript/ql/test/experimental/Security/CWE-020/RegexValidation.qlref

@@ -0,0 +1 @@
+experimental/Security/CWE-020/RegexValidation.ql