Skip to content

Commit 2061859

Browse files
r1tsuuciechanowiec
andauthored
fix: escape regex metacharacters in isURLAllowed pathname allow-list (#17237)
Port of #16997 to `3.x` Co-authored-by: Herman Ciechanowiec <herman@ciechanowiec.eu>
1 parent f31f26d commit 2061859

3 files changed

Lines changed: 87 additions & 7 deletions

File tree

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
import { describe, expect, it } from 'vitest'
2+
3+
import type { AllowList } from '../uploads/types.js'
4+
5+
import { isURLAllowed } from './isURLAllowed.js'
6+
7+
describe('isURLAllowed', () => {
8+
describe('hostname matching', () => {
9+
const allowList: AllowList = [{ hostname: 'cdn.example.com' }]
10+
11+
it('should allow an exactly matching hostname', () => {
12+
expect(isURLAllowed('https://cdn.example.com/file.png', allowList)).toBe(true)
13+
})
14+
15+
it('should deny a different hostname', () => {
16+
expect(isURLAllowed('https://attacker.com/file.png', allowList)).toBe(false)
17+
})
18+
19+
it('should deny a userinfo `@` trick (hostname is the real authority)', () => {
20+
expect(isURLAllowed('https://cdn.example.com@attacker.com/file.png', allowList)).toBe(false)
21+
})
22+
23+
it('should deny an invalid URL', () => {
24+
expect(isURLAllowed('not a url', allowList)).toBe(false)
25+
})
26+
})
27+
28+
describe('pathname matching', () => {
29+
it('should treat a literal dot as a literal, not a wildcard', () => {
30+
const allowList: AllowList = [{ hostname: 'cdn.example.com', pathname: '/files/report.json' }]
31+
32+
expect(isURLAllowed('https://cdn.example.com/files/report.json', allowList)).toBe(true)
33+
// Previously the unescaped `.` matched any character, widening the allow-list.
34+
expect(isURLAllowed('https://cdn.example.com/files/reportXjson', allowList)).toBe(false)
35+
})
36+
37+
it('should not let other regex metacharacters broaden the match', () => {
38+
const allowList: AllowList = [{ hostname: 'cdn.example.com', pathname: '/a+b/(c)' }]
39+
40+
expect(isURLAllowed('https://cdn.example.com/a+b/(c)', allowList)).toBe(true)
41+
expect(isURLAllowed('https://cdn.example.com/aaab/c', allowList)).toBe(false)
42+
})
43+
44+
it('should match a single segment with `*` but not across slashes', () => {
45+
const allowList: AllowList = [{ hostname: 'cdn.example.com', pathname: '/uploads/*' }]
46+
47+
expect(isURLAllowed('https://cdn.example.com/uploads/photo.png', allowList)).toBe(true)
48+
expect(isURLAllowed('https://cdn.example.com/uploads/nested/photo.png', allowList)).toBe(
49+
false,
50+
)
51+
})
52+
53+
it('should match across slashes with `**`', () => {
54+
const allowList: AllowList = [{ hostname: 'cdn.example.com', pathname: '/uploads/**' }]
55+
56+
expect(isURLAllowed('https://cdn.example.com/uploads/photo.png', allowList)).toBe(true)
57+
expect(isURLAllowed('https://cdn.example.com/uploads/nested/photo.png', allowList)).toBe(true)
58+
})
59+
60+
it('should allow an optional trailing slash', () => {
61+
const allowList: AllowList = [{ hostname: 'cdn.example.com', pathname: '/assets/' }]
62+
63+
expect(isURLAllowed('https://cdn.example.com/assets', allowList)).toBe(true)
64+
expect(isURLAllowed('https://cdn.example.com/assets/', allowList)).toBe(true)
65+
})
66+
})
67+
})

packages/payload/src/utilities/isURLAllowed.ts

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
import type { AllowList } from '../uploads/types.js'
22

3+
import { escapeRegExp } from './escapeRegExp.js'
4+
35
export const isURLAllowed = (url: string, allowList: AllowList): boolean => {
46
try {
57
const parsedUrl = new URL(url)
@@ -16,10 +18,15 @@ export const isURLAllowed = (url: string, allowList: AllowList): boolean => {
1618
}
1719

1820
if (key === 'pathname') {
19-
// Convert wildcards to a regex
20-
const regexPattern = value
21-
.replace(/\*\*/g, '.*') // Match any path
22-
.replace(/\*/g, '[^/]*') // Match any part of a path segment
21+
// Translate a small glob syntax to a regex. The pattern is escaped
22+
// first so that metacharacters in the configured value (e.g. `.`)
23+
// match literally and cannot broaden what the allow-list accepts.
24+
// Wildcards become `\*` once escaped, so they are restored afterwards
25+
// — translating `**` before `*` is safe because the resulting `.*`
26+
// no longer contains an escaped `\*` for the next replace to match.
27+
const regexPattern = escapeRegExp(value)
28+
.replace(/\\\*\\\*/g, '.*') // `**` → match any path
29+
.replace(/\\\*/g, '[^/]*') // `*` → match any part of a path segment
2330
.replace(/\/$/, '(/)?') // Allow optional trailing slash
2431
const regex = new RegExp(`^${regexPattern}$`)
2532
return regex.test(parsedUrl.pathname)

packages/ui/src/utilities/isURLAllowed.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,16 @@ export const isURLAllowed = (url: string, allowList: AllowList): boolean => {
1616
}
1717

1818
if (key === 'pathname') {
19-
// Convert wildcards to a regex
19+
// Translate a small glob syntax to a regex. The pattern is escaped
20+
// first so that metacharacters in the configured value (e.g. `.`)
21+
// match literally and cannot broaden what the allow-list accepts.
22+
// Wildcards become `\*` once escaped, so they are restored afterwards
23+
// — translating `**` before `*` is safe because the resulting `.*`
24+
// no longer contains an escaped `\*` for the next replace to match.
2025
const regexPattern = value
21-
.replace(/\*\*/g, '.*') // Match any path
22-
.replace(/\*/g, '[^/]*') // Match any part of a path segment
26+
.replace(/[\\^$*+?.()|[\]{}]/g, '\\$&') // Escape regex metacharacters
27+
.replace(/\\\*\\\*/g, '.*') // `**` → match any path
28+
.replace(/\\\*/g, '[^/]*') // `*` → match any part of a path segment
2329
const regex = new RegExp(`^${regexPattern}$`)
2430
return regex.test(parsedUrl.pathname)
2531
}

0 commit comments

Comments
 (0)