Javascript: Regex Global Flag in Test Function

[aydinnyunus]

Pull Request: Add Regex Global Flag in Test Function Query for CodeQL

Overview

This pull request adds a new CodeQL query designed to detect issues related to the use of the global flag (g) in regular expressions within JavaScript and TypeScript codebases. This query focuses on identifying instances where the global flag might lead to inconsistent or erroneous behavior, particularly when used in conjunction with the test method of RegExp objects. The goal is to help developers identify and rectify potential bugs in their code related to global regular expressions.

Changes Introduced

• New Query Added: RegexValidation.ql - Identifies potentially problematic uses of the global flag in regular expressions, especially when used in test method calls.
• Code Examples: Provided in the /CWE-020 directory, showcasing both problematic (bad) and corrected (good) usage scenarios.
• Documentation: Comprehensive documentation detailing the query's purpose, how it works, and the specific issue it addresses, including examples and best practices.

Implementation Details

• The query targets JavaScript and TypeScript files, scanning for RegExp literals with a global flag and their use in test method calls.
• Special attention is given to scenarios where the regex object's state might lead to unexpected results due to the persistence of the lastIndex property.

Testing and Validation

• Rigorously tested using a variety of synthetic code samples, illustrating common pitfalls and edge cases.
• Performance and accuracy assessed to minimize false positives and ensure efficiency in large codebases.

Future Work

• Plans to refine the query based on user feedback and evolving best practices.
• Consider extending the query's scope to cover more complex scenarios or additional programming languages where similar patterns may be relevant.

References

• MDN Documentation on Regular Expressions: MDN RegExp.
• Discussion on the nuances of the global flag in JavaScript regular expressions: Global Flag in JavaScript RegEx.

[aydinnyunus]

I will create issue on securitylab after approve 🙏

[aydinnyunus]

Rule is checking following misconfiguration.

[github-actions]

QHelp previews:

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

Regex Global Flag in Test Function

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.

Recommendation

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.

Example

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

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);

Example

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

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);

References

• MDN Web Docs - Regular Expressions: Understanding Regular Expressions in JavaScript Example Stackoverflow Discussion

[erik-krogh]

I will create issue on securitylab after approve 🙏

Please create that issue first. It makes the process for us easier.

[aydinnyunus]

@erik-krogh I created the issue thank you.

github/securitylab#810

[ghsecuritylab]

Hello aydinnyunus 👋
You have submitted this pull request as a bug bounty report in the github/securitylab repository and therefore this pull request has been put into draft state to give time for the GitHub Security Lab to assess the PR. When GitHub Security Lab has finished assessing your pull request, it will be marked automatically as Ready for review. Until then, please don't change the draft state.

In the meantime, feel free to make changes to the pull request. If you'd like to maximize payout for your this and future submissions, here are a few general guidelines, that we might take into consideration when reviewing a submission.

• the submission models widely-used frameworks/libraries
• the vulnerability modeled in the submission is impactful
• the submission finds new true positive vulnerabilities
• the submission finds very few false positives
• code in the submission is easy to read and will be easy to maintain
• documentation is written clearly, highlighting the impact of the issue it finds and is written without grammatical or other errors. The code samples clearly show the vulnerability
• the submission includes tests, change note etc.

Please note that these are guidelines, not rules. Since we have a lot of different types of submissions, the guidelines might vary for each submission.

Happy hacking!

[aydinnyunus]

Thank you. I am waiting for your answer <3