Skip to content

Fix polynomial regular expressions used on user controlled data#5857

Merged
christopherholland-workday merged 3 commits into
mainfrom
polynomial-regex
Feb 27, 2026
Merged

Fix polynomial regular expressions used on user controlled data#5857
christopherholland-workday merged 3 commits into
mainfrom
polynomial-regex

Conversation

@christopherholland-workday
Copy link
Copy Markdown
Contributor

@christopherholland-workday christopherholland-workday commented Feb 27, 2026

Fixes for CodeQL findings 76-79.

Uncontrolled data is passed into regex's that are un-optimal which could result in a DoS due to poor performance.

This PR creates unit tests that worked for the original regex's and updates the regex's to be safer.

Line Before After Why
1239 ![.?](….?) New sanitation method See below
1459 {\s*([^}]+)\s*} {([^}]+)} Removes \s* padding that overlapped with [^}]+; .trim() on the captured value already handles whitespace
1579 \s+.*= \s+\S[^=]*= \S forces \s+ to consume all leading whitespace unambiguously; [^=]* can't match =
1596 import\s+.*?\s+from import\s+\S[^\n]*?\s+from Same \S anchor trick — adversarial all-whitespace input fails in O(n) instead of O(n²)

Line 1239:
Why regex can't be made safe here (without possessive quantifiers):

The problem isn't the backtracking within the regex — it's the g flag forcing the engine to retry at every ! character. With n occurrences of ![ and each requiring a scan of up to O(n) chars to find ], the total is O(n²) regardless of whether .? or [^\]] is used. JavaScript has no possessive quantifiers ([^\]]*+) or atomic groups ((?>...)) to prevent this.

What the new implementation does differently:

Each indexOf call scans forward and never rescans already-visited characters. The total work across the whole loop is O(n) — linear regardless of how many ![ sequences appear in the input.

Testing

  1. Created the unit tests in packages/components/src/utils.test.ts
  2. Ran them against the original code -> Passed
  3. Made the regex changes
  4. Ran unit tests against changes -> Passed
 PASS  test/utils.test.ts (5.448 s)
  removeInvalidImageMarkdown
    strips non-http/https image markdown
      ✓ removes a relative-path image (1 ms)
      ✓ removes an image with no URL scheme
      ✓ removes a data-URI image
      ✓ removes an image with an absolute local path
    preserves http and https image markdown
      ✓ keeps an https image
      ✓ keeps an http image
    non-string inputs pass through unchanged
      ✓ returns null as-is (3 ms)
      ✓ returns a number as-is
      ✓ returns an object as-is
      ✓ returns undefined as-is
    mixed content in the same string
      ✓ strips relative image but keeps https image (1 ms)
      ✓ strips multiple non-http images
      ✓ preserves surrounding text
    edge cases
      ✓ handles empty string
      ✓ handles string with no images
      ✓ does not remove a link that is not an image (missing !)
  convertRequireToImport
    default require → default import
      ✓ converts const default require
      ✓ converts let default require (1 ms)
      ✓ converts var default require
      ✓ handles scoped package names
    destructured require → named import
      ✓ converts single destructured require
      ✓ converts multiple destructured require
      ✓ trims outer whitespace from destructured vars
    property-access require
      ✓ matches as a default import (default pattern takes precedence; .property is not captured)
    indentation preservation
      ✓ preserves leading spaces
      ✓ preserves leading tab
    unrecognised input returns null
      ✓ returns null for a console.log call (1 ms)
      ✓ returns null when require is not called
      ✓ returns null for an empty string
  CommonJS detection regex (utils.ts line 1579 pattern)
    ✓ matches a const default require
    ✓ matches a let default require
    ✓ matches a destructured require
    ✓ does not match a non-require assignment
    ✓ does not match an import statement
    ✓ does not match a bare require call (no assignment)
  Import extraction regex (utils.ts line 1596 pattern)
    ✓ extracts module from a default import (1 ms)
    ✓ extracts module from a named import
    ✓ extracts module from a namespace import
    ✓ extracts module from a require call
    ✓ extracts module from a require call inside an assignment
    ✓ extracts a scoped package name
    ✓ extracts multiple modules from a multi-line code block
    ✓ returns empty array when there are no imports

Test Suites: 1 passed, 1 total
Tests:       43 passed, 43 total
Snapshots:   0 total
Time:        5.542 s

@gemini-code-assist
Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the application's security posture by mitigating ReDoS vulnerabilities. It refines several regular expressions that previously processed untrusted input, ensuring they operate efficiently and safely. The changes are thoroughly validated with new unit tests, providing robust coverage for the updated patterns and utility functions.

Highlights

  • Security Fix: Addressed CodeQL findings 76-79 by updating several regular expressions to prevent potential Regular Expression Denial of Service (ReDoS) vulnerabilities when processing user-controlled data.
  • Regex Optimization: Modified four specific regular expressions to use more efficient patterns (e.g., [^]]* instead of .*?) to avoid catastrophic backtracking and improve performance.
  • New Unit Tests: Introduced comprehensive unit tests for the removeInvalidImageMarkdown and convertRequireToImport utility functions, as well as for the CommonJS detection and import extraction regex patterns, ensuring the correctness and safety of the updated logic.
Changelog
  • packages/components/src/utils.test.ts
    • Added new test suite for removeInvalidImageMarkdown covering various scenarios including stripping non-HTTP/HTTPS images, preserving valid ones, handling non-string inputs, mixed content, and edge cases.
    • Added new test suite for convertRequireToImport covering default and destructured require conversions, property-access, indentation preservation, and unrecognised inputs.
    • Added new test suite for the CommonJS detection regex used in executeJavaScriptCode to verify its matching behavior.
    • Added new test suite for the import extraction regex used in executeJavaScriptCode to verify its module extraction capabilities.
  • packages/components/src/utils.ts
    • Updated the regular expression in removeInvalidImageMarkdown from !\[.*?\].*? to !\[[^\]]*\].*? to prevent catastrophic backtracking.
    • Exported the convertRequireToImport helper function to make it testable.
    • Updated the regular expression for destructureMatch in convertRequireToImport from \{\s*([^}]+)\s*\} to \{([^}]+)\} to remove redundant whitespace matching, as trimming is handled separately.
    • Updated the CommonJS detection regular expression in executeJavaScriptCode from \s+.*= to \s+\S[^=]*= to improve efficiency and prevent ReDoS.
    • Updated the import extraction regular expression in executeJavaScriptCode from import\s+.*?\s+from to import\s+\S[^\n]*?\s+from to improve efficiency and prevent ReDoS.
Activity
  • Unit tests were created in packages/components/src/utils.test.ts.
  • The newly created unit tests were run against the original code and passed.
  • The necessary regex changes were implemented.
  • The unit tests were re-run against the modified code and passed, confirming the fixes.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Comment thread packages/components/src/utils.ts Fixed
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a new test file (packages/components/src/utils.test.ts) that adds comprehensive unit tests for the removeInvalidImageMarkdown and convertRequireToImport utility functions, as well as for two regular expressions used for CommonJS detection and import extraction within executeJavaScriptCode in packages/components/src/utils.ts. The code changes in utils.ts include refining the regex patterns for removeInvalidImageMarkdown, convertRequireToImport's destructured match, and the CommonJS detection and import extraction regexes. Additionally, the convertRequireToImport function is now exported. A review comment suggests improving maintainability by exporting the CommonJS detection and import extraction regular expressions from utils.ts and importing them into the test file, rather than duplicating them, to ensure tests always validate the exact regexes used in the application.

Comment thread packages/components/src/utils.test.ts Outdated
@christopherholland-workday christopherholland-workday merged commit af2d2f6 into main Feb 27, 2026
7 checks passed
@yau-wd yau-wd deleted the polynomial-regex branch March 3, 2026 08:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants