Skip to content

Commit f7a2733

Browse files
committed
fix(security): replace RegExp with string methods to prevent ReDoS in route matching
1 parent 14d4d88 commit f7a2733

File tree

6 files changed

+249
-4
lines changed

6 files changed

+249
-4
lines changed

cspell-dictionaries/project.txt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
Descriptionless
2+
languagedetector
3+
Lngs
24
Pipeable
5+
Platzhalterseite
36
plex
47
Stephane
8+
Testbarkeit
59
fouc
610
vite

cspell.json

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,13 @@
1212
"path": "./cspell-dictionaries/project.txt"
1313
}
1414
],
15+
"import": ["@cspell/dict-de-de/cspell-ext.json"],
16+
"overrides": [
17+
{
18+
"filename": "**/locales/*.json",
19+
"language": "en,de"
20+
}
21+
],
1522
"enabledLanguageIds": [
1623
"git-commit",
1724
"graphql",

package-lock.json

Lines changed: 8 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
"sirv": "^3.0.0"
4444
},
4545
"devDependencies": {
46+
"@cspell/dict-de-de": "^4.1.2",
4647
"@double-great/stylelint-a11y": "^3.0.4",
4748
"@eslint/js": "^9.21.0",
4849
"@testing-library/dom": "^10.4.1",

src/__tests__/routes.config.test.js

Lines changed: 196 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,17 @@
11
/**
2-
* Copyright IBM Corp. 2025
2+
* Copyright IBM Corp. 2025, 2026
33
*
44
* This source code is licensed under the Apache-2.0 license found in the
55
* LICENSE file in the root directory of this source tree.
66
*/
77

88
import { test, expect, describe } from 'vitest';
9-
import { routes, routesInHeader, routesInSideNav } from '../routes/config';
9+
import {
10+
routes,
11+
routesInHeader,
12+
routesInSideNav,
13+
isDirectChildPath,
14+
} from '../routes/config';
1015
import Dashboard from '../pages/dashboard/Dashboard';
1116
import NotFound from '../pages/not-found/NotFound';
1217
import Placeholder from '../pages/placeholder/Placeholder';
@@ -113,3 +118,192 @@ describe('routes configuration', () => {
113118
});
114119
});
115120
});
121+
122+
describe('isDirectChildPath', () => {
123+
test('returns true for direct child paths', () => {
124+
expect(isDirectChildPath('/link-4', '/link-4/sub-link-1')).toBe(true);
125+
expect(isDirectChildPath('/dashboard', '/dashboard/123')).toBe(true);
126+
expect(isDirectChildPath('/api', '/api/users')).toBe(true);
127+
expect(isDirectChildPath('/a', '/a/b')).toBe(true);
128+
});
129+
130+
test('returns false for nested child paths (grandchildren)', () => {
131+
expect(isDirectChildPath('/link-4', '/link-4/sub-link-1/nested')).toBe(
132+
false,
133+
);
134+
expect(isDirectChildPath('/dashboard', '/dashboard/123/details')).toBe(
135+
false,
136+
);
137+
expect(isDirectChildPath('/api', '/api/users/profile')).toBe(false);
138+
expect(isDirectChildPath('/a', '/a/b/c')).toBe(false);
139+
});
140+
141+
test('returns false when paths are identical', () => {
142+
expect(isDirectChildPath('/dashboard', '/dashboard')).toBe(false);
143+
expect(isDirectChildPath('/link-4', '/link-4')).toBe(false);
144+
expect(isDirectChildPath('/', '/')).toBe(false);
145+
});
146+
147+
test('returns false for non-matching paths', () => {
148+
expect(isDirectChildPath('/link-4', '/link-5')).toBe(false);
149+
expect(isDirectChildPath('/dashboard', '/profile')).toBe(false);
150+
expect(isDirectChildPath('/api', '/app')).toBe(false);
151+
});
152+
153+
test('returns false for paths that start similarly but are not children', () => {
154+
expect(isDirectChildPath('/link', '/link-4')).toBe(false);
155+
expect(isDirectChildPath('/dash', '/dashboard')).toBe(false);
156+
expect(isDirectChildPath('/api', '/api-v2')).toBe(false);
157+
});
158+
159+
test('handles edge cases with null, undefined, and empty strings', () => {
160+
expect(isDirectChildPath(null, '/path')).toBe(false);
161+
expect(isDirectChildPath('/path', null)).toBe(false);
162+
expect(isDirectChildPath(undefined, '/path')).toBe(false);
163+
expect(isDirectChildPath('/path', undefined)).toBe(false);
164+
expect(isDirectChildPath('', '/path')).toBe(false);
165+
expect(isDirectChildPath('/path', '')).toBe(false);
166+
expect(isDirectChildPath('', '')).toBe(false);
167+
});
168+
169+
test('handles paths with special characters', () => {
170+
expect(isDirectChildPath('/user-profile', '/user-profile/settings')).toBe(
171+
true,
172+
);
173+
expect(isDirectChildPath('/api_v2', '/api_v2/endpoint')).toBe(true);
174+
expect(isDirectChildPath('/path.name', '/path.name/child')).toBe(true);
175+
});
176+
177+
test('handles trailing slashes correctly', () => {
178+
// Parent with trailing slash should not match (different path structure)
179+
expect(isDirectChildPath('/link-4/', '/link-4/sub-link-1')).toBe(false);
180+
// Child with trailing slash contains '/' in remainder, so not a direct child
181+
expect(isDirectChildPath('/link-4', '/link-4/sub-link-1/')).toBe(false);
182+
// Without trailing slashes works correctly
183+
expect(isDirectChildPath('/link-4', '/link-4/sub-link-1')).toBe(true);
184+
});
185+
186+
test('is case-sensitive', () => {
187+
expect(isDirectChildPath('/Link-4', '/link-4/sub-link-1')).toBe(false);
188+
expect(isDirectChildPath('/link-4', '/Link-4/sub-link-1')).toBe(false);
189+
});
190+
191+
test('works with root path', () => {
192+
// Root path '/' + '/' = '//', so '/dashboard' doesn't start with '//'
193+
// This is expected behavior - root path needs special handling if needed
194+
expect(isDirectChildPath('/', '/dashboard')).toBe(false);
195+
196+
// For actual use case, routes don't use '/' as parent in the config
197+
// But if they did, this would be the correct behavior
198+
expect(isDirectChildPath('/', '//dashboard')).toBe(true);
199+
expect(isDirectChildPath('/', '//dashboard/123')).toBe(false);
200+
});
201+
202+
test('handles URL parameters correctly', () => {
203+
// Dynamic route parameters (common in React Router)
204+
expect(isDirectChildPath('/dashboard', '/dashboard/:id')).toBe(true);
205+
expect(isDirectChildPath('/users', '/users/:userId')).toBe(true);
206+
expect(isDirectChildPath('/api', '/api/:version')).toBe(true);
207+
208+
// Multiple parameters
209+
expect(isDirectChildPath('/users/:userId', '/users/:userId/posts')).toBe(
210+
true,
211+
);
212+
expect(
213+
isDirectChildPath('/users/:userId/posts', '/users/:userId/posts/:postId'),
214+
).toBe(true);
215+
216+
// Should not match nested parameters
217+
expect(isDirectChildPath('/users', '/users/:userId/posts/:postId')).toBe(
218+
false,
219+
);
220+
});
221+
222+
test('handles query strings and hash fragments', () => {
223+
// Query strings should be treated as part of the path
224+
expect(isDirectChildPath('/search', '/search/results?q=test')).toBe(true);
225+
expect(isDirectChildPath('/api', '/api/data?format=json')).toBe(true);
226+
227+
// Hash fragments
228+
expect(isDirectChildPath('/docs', '/docs/intro#section')).toBe(true);
229+
230+
// Combined
231+
expect(isDirectChildPath('/page', '/page/view?id=1#top')).toBe(true);
232+
233+
// Should not match if there's a nested path before query/hash
234+
expect(isDirectChildPath('/api', '/api/v1/data?format=json')).toBe(false);
235+
});
236+
237+
test('handles special URL characters', () => {
238+
// Encoded characters
239+
expect(isDirectChildPath('/search', '/search/hello%20world')).toBe(true);
240+
expect(isDirectChildPath('/files', '/files/document.pdf')).toBe(true);
241+
242+
// Special characters that are valid in URLs
243+
expect(isDirectChildPath('/items', '/items/item-123')).toBe(true);
244+
expect(isDirectChildPath('/tags', '/tags/tag_name')).toBe(true);
245+
expect(isDirectChildPath('/docs', '/docs/v1.0.0')).toBe(true);
246+
expect(isDirectChildPath('/api', '/api/~user')).toBe(true);
247+
248+
// Parentheses (sometimes used in routing)
249+
expect(isDirectChildPath('/routes', '/routes/(auth)')).toBe(true);
250+
251+
// Plus sign
252+
expect(isDirectChildPath('/search', '/search/C++')).toBe(true);
253+
});
254+
255+
test('handles wildcard and catch-all patterns', () => {
256+
// Asterisk (catch-all routes)
257+
expect(isDirectChildPath('/files', '/files/*')).toBe(true);
258+
expect(isDirectChildPath('/docs', '/docs/*')).toBe(true);
259+
260+
// Should not match nested wildcards
261+
expect(isDirectChildPath('/files', '/files/subfolder/*')).toBe(false);
262+
});
263+
264+
test('handles optional segments notation', () => {
265+
// Optional segments (React Router syntax)
266+
expect(isDirectChildPath('/users', '/users/:id?')).toBe(true);
267+
expect(isDirectChildPath('/posts', '/posts/:slug?')).toBe(true);
268+
269+
// Splat parameters
270+
expect(isDirectChildPath('/files', '/files/*filepath')).toBe(true);
271+
});
272+
273+
test('handles real-world routing scenarios', () => {
274+
// API versioning
275+
expect(isDirectChildPath('/api', '/api/v1')).toBe(true);
276+
expect(isDirectChildPath('/api', '/api/v1/users')).toBe(false);
277+
278+
// Locale routing
279+
expect(isDirectChildPath('/en', '/en/dashboard')).toBe(true);
280+
expect(isDirectChildPath('/en', '/en/dashboard/settings')).toBe(false);
281+
282+
// Admin routes
283+
expect(isDirectChildPath('/admin', '/admin/users')).toBe(true);
284+
expect(isDirectChildPath('/admin', '/admin/users/edit')).toBe(false);
285+
286+
// Resource routes (RESTful)
287+
expect(isDirectChildPath('/posts', '/posts/new')).toBe(true);
288+
expect(isDirectChildPath('/posts', '/posts/:id')).toBe(true);
289+
expect(isDirectChildPath('/posts/:id', '/posts/:id/edit')).toBe(true);
290+
expect(isDirectChildPath('/posts/:id', '/posts/:id/comments')).toBe(true);
291+
expect(
292+
isDirectChildPath('/posts/:id', '/posts/:id/comments/:commentId'),
293+
).toBe(false);
294+
});
295+
296+
test('prevents ReDoS by avoiding regex', () => {
297+
// These patterns would be problematic with regex but are safe with string methods
298+
const maliciousPath = '/a'.repeat(100);
299+
const maliciousSubPath = maliciousPath + '/b';
300+
301+
// Should complete quickly without hanging
302+
const startTime = Date.now();
303+
const result = isDirectChildPath(maliciousPath, maliciousSubPath);
304+
const endTime = Date.now();
305+
306+
expect(result).toBe(true);
307+
expect(endTime - startTime).toBeLessThan(10); // Should be nearly instant
308+
});
309+
});

src/routes/config.js

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,38 @@ export const routes = [
160160
},
161161
];
162162

163+
/**
164+
* Checks if a subPath is a direct child of a parent path.
165+
* A direct child means the subPath starts with the parent path followed by '/',
166+
* and contains no additional '/' characters after that.
167+
*
168+
* @param {string} parentPath - The parent path to check against
169+
* @param {string} subPath - The potential child path
170+
* @returns {boolean} True if subPath is a direct child of parentPath
171+
*
172+
* @example
173+
* isDirectChildPath('/link-4', '/link-4/sub-link-1') // true
174+
* isDirectChildPath('/link-4', '/link-4/sub-link-1/nested') // false
175+
* isDirectChildPath('/dashboard', '/dashboard/123') // true
176+
* isDirectChildPath('/dashboard', '/dashboard') // false
177+
*/
178+
export const isDirectChildPath = (parentPath, subPath) => {
179+
if (!subPath || !parentPath) {
180+
return false;
181+
}
182+
183+
// Check if subPath starts with parentPath followed by '/'
184+
if (!subPath.startsWith(parentPath + '/')) {
185+
return false;
186+
}
187+
188+
// Get the remainder after the parent path and separator
189+
const remainder = subPath.slice(parentPath.length + 1);
190+
191+
// A direct child should have content but no additional '/' characters
192+
return remainder.length > 0 && !remainder.includes('/');
193+
};
194+
163195
// The routes config is a flat structure defined for use with react-router.
164196
// Here we organize the routes into a hierarchy for use by the Carbon header and sidenav
165197
// NOTE: The routes are processed outside of a component as they are not dynamic.
@@ -175,9 +207,8 @@ const routesProcessed = routes.map((route) => {
175207
if (!subRoute.carbon) return false;
176208

177209
const subPath = subRoute.path || subRoute.carbon.virtualPath;
178-
const childPath = new RegExp(`^${path}/[^/]+$`); // match direct parent only
179210

180-
return !route.index && subPath && childPath.test(subPath);
211+
return !route.index && isDirectChildPath(path, subPath);
181212
});
182213

183214
if (subMenu && subMenu.length > 0) {

0 commit comments

Comments
 (0)