Skip to content

fix(urlMatcher): Fix potential ReDoS, thanks to www.HeroDevs.com #849

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
christopherthielen merged 2 commits into from
Dec 30, 2025
+10 −6
Merged

fix(urlMatcher): Fix potential ReDoS, thanks to www.HeroDevs.com #849

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
66bd656
fix(urlMatcher): Fix potential ReDoS, thanks to www.HeroDevs.com
christopherthielen Sep 5, 2025
7c2f1a4
chore: use openssl-legacy-provider to fix CI
christopherthielen Dec 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ on:
- master
pull_request:

env:
NODE_OPTIONS: --openssl-legacy-provider

jobs:
ci:
needs: [test, downstream]
Expand Down
13 changes: 7 additions & 6 deletions src/url/urlMatcher.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,13 +234,14 @@ export class UrlMatcher {
// The regular expression is somewhat complicated due to the need to allow curly braces
// inside the regular expression. The placeholder regexp breaks down as follows:
// ([:*])([\w\[\]]+) - classic placeholder ($1 / $2) (search version has - for snake-case)
// \{([\w\[\]]+)(?:\:\s*( ... ))?\} - curly brace placeholder ($3) with optional regexp/type ... ($4) (search version has - for snake-case
// \{([\w\[\]]+)(?:\: ... ( ... ))?\} - curly brace placeholder ($3) with optional regexp/type ... ($5) (search version has - for snake-case
// (?: ... | ... | ... )+ - the regexp consists of any number of atoms, an atom being either
// [^{}\\]+ - anything other than curly braces or backslash
// [^{}\\] - anything other than curly braces or backslash
Copy link

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

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

The comment describes the pattern as matching a single character, but this doesn't accurately reflect how it's used in the actual regex where it appears within a group with quantifiers like +. The comment should clarify that this represents a single character atom that can be repeated within the (?:...)+ construct.

Suggested change
// [^{}\\] - anything other than curly braces or backslash
// [^{}\\] - a single-character atom: any character other than curly braces or backslash

Copilot uses AI. Check for mistakes.
// \\. - a backslash escape
// \{(?:[^{}\\]+|\\.)*\} - a matched set of curly braces containing other atoms
const placeholder = /([:*])([\w\[\]]+)|\{([\w\[\]]+)(?:\:\s*((?:[^{}\\]+|\\.|\{(?:[^{}\\]+|\\.)*\})+))?\}/g;
const searchPlaceholder = /([:]?)([\w\[\].-]+)|\{([\w\[\].-]+)(?:\:\s*((?:[^{}\\]+|\\.|\{(?:[^{}\\]+|\\.)*\})+))?\}/g;
// \{(?:[^{}\\]|\\.)*\} - a matched set of curly braces containing other atoms
const placeholder = /([:*])([\w\[\]]+)|\{([\w\[\]]+)(?:\:(?=(\s*))\4((?:[^{}\\]|\\.|\{(?:[^{}\\]|\\.)*\})+))?\}/g;
const searchPlaceholder =
/([:]?)([\w\[\].-]+)|\{([\w\[\].-]+)(?:\:(?=(\s*))\4((?:[^{}\\]|\\.|\{(?:[^{}\\]|\\.)*\})+))?\}/g;
Comment on lines +242 to +244
Copy link

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

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

The backreference \4 refers to capture group 4 which captures optional whitespace via (\s*). This creates a dependency where the exact whitespace captured must be repeated, which may not be the intended behavior. Consider using a non-capturing group (?:\s*) instead of the lookahead-backreference pattern if the goal is simply to allow optional whitespace after the colon.

Suggested change
const placeholder = /([:*])([\w\[\]]+)|\{([\w\[\]]+)(?:\:(?=(\s*))\4((?:[^{}\\]|\\.|\{(?:[^{}\\]|\\.)*\})+))?\}/g;
const searchPlaceholder =
/([:]?)([\w\[\].-]+)|\{([\w\[\].-]+)(?:\:(?=(\s*))\4((?:[^{}\\]|\\.|\{(?:[^{}\\]|\\.)*\})+))?\}/g;
const placeholder = /([:*])([\w\[\]]+)|\{([\w\[\]]+)(?:\:(\s*)((?:[^{}\\]|\\.|\{(?:[^{}\\]|\\.)*\})+))?\}/g;
const searchPlaceholder =
/([:]?)([\w\[\].-]+)|\{([\w\[\].-]+)(?:\:(\s*)((?:[^{}\\]|\\.|\{(?:[^{}\\]|\\.)*\})+))?\}/g;

Copilot uses AI. Check for mistakes.
const patterns: any[][] = [];
let last = 0;
let matchArray: RegExpExecArray;
Expand All @@ -256,7 +257,7 @@ export class UrlMatcher {
const matchDetails = (m: RegExpExecArray, isSearch: boolean): MatchDetails => {
// IE[78] returns '' for unmatched groups instead of null
const id: string = m[2] || m[3];
const regexp: string = isSearch ? m[4] : m[4] || (m[1] === '*' ? '[\\s\\S]*' : null);
const regexp: string = isSearch ? m[5] : m[5] || (m[1] === '*' ? '[\\s\\S]*' : null);

const makeRegexpType = (str) =>
inherit(paramTypes.type(isSearch ? 'query' : 'path'), {
Expand Down