diff --git a/javascript/ql/src/experimental/Security/CWE-020/RegexGlobalFlagBad.ts b/javascript/ql/src/experimental/Security/CWE-020/RegexGlobalFlagBad.ts new file mode 100644 index 000000000000..96149e656e53 --- /dev/null +++ b/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); \ No newline at end of file diff --git a/javascript/ql/src/experimental/Security/CWE-020/RegexGlobalFlagGood.ts b/javascript/ql/src/experimental/Security/CWE-020/RegexGlobalFlagGood.ts new file mode 100644 index 000000000000..cf002bd19484 --- /dev/null +++ b/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); \ No newline at end of file diff --git a/javascript/ql/src/experimental/Security/CWE-020/RegexValidation.qhelp b/javascript/ql/src/experimental/Security/CWE-020/RegexValidation.qhelp new file mode 100644 index 000000000000..03aed8b2203c --- /dev/null +++ b/javascript/ql/src/experimental/Security/CWE-020/RegexValidation.qhelp @@ -0,0 +1,33 @@ + + + +

+ 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. +

+
+ +

+ 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. +

+
+ +

+ Vulnerable code example: Using a global regex in a test function without resetting lastIndex. The function may return inconsistent results over repeated calls. +

+ +
+ +

+ Secure code example: Resetting the lastIndex of the regex to 0 before each test call or using match for global searches. +

+ +
+ +
  • + MDN Web Docs - Regular Expressions: + Understanding Regular Expressions in JavaScript + Example Stackoverflow Discussion +
  • + +
    +
    diff --git a/javascript/ql/src/experimental/Security/CWE-020/RegexValidation.ql b/javascript/ql/src/experimental/Security/CWE-020/RegexValidation.ql new file mode 100644 index 000000000000..da7b50f34764 --- /dev/null +++ b/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." diff --git a/javascript/ql/test/experimental/Security/CWE-020/RegexGlobalFlagBad.ts b/javascript/ql/test/experimental/Security/CWE-020/RegexGlobalFlagBad.ts new file mode 100644 index 000000000000..96149e656e53 --- /dev/null +++ b/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); \ No newline at end of file diff --git a/javascript/ql/test/experimental/Security/CWE-020/RegexGlobalFlagGood.ts b/javascript/ql/test/experimental/Security/CWE-020/RegexGlobalFlagGood.ts new file mode 100644 index 000000000000..cf002bd19484 --- /dev/null +++ b/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); \ No newline at end of file diff --git a/javascript/ql/test/experimental/Security/CWE-020/RegexValidation.expected b/javascript/ql/test/experimental/Security/CWE-020/RegexValidation.expected new file mode 100644 index 000000000000..1e182563cda7 --- /dev/null +++ b/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. | diff --git a/javascript/ql/test/experimental/Security/CWE-020/RegexValidation.qlref b/javascript/ql/test/experimental/Security/CWE-020/RegexValidation.qlref new file mode 100644 index 000000000000..ba71ba3dea11 --- /dev/null +++ b/javascript/ql/test/experimental/Security/CWE-020/RegexValidation.qlref @@ -0,0 +1 @@ +experimental/Security/CWE-020/RegexValidation.ql \ No newline at end of file