Skip to content

Commit 66cfce2

Browse files
committed
fix(security): harden pattern matcher and qualifier validation
- Cap wildcards per pattern in matchWildcard to prevent polynomial backtracking DoS via many alternating `*`/`?` within the overall length limit. - Reject non-string qualifier keys in validateQualifiers (throws PurlError when throws=true) and skip them in normalizeQualifiers, closing a gap where custom keys iterators could yield non-strings.
1 parent 99c8088 commit 66cfce2

4 files changed

Lines changed: 131 additions & 0 deletions

File tree

src/compare.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,12 +54,30 @@ const WILDCARD_CACHE_MAX = 1024
5454
* Designed for version strings and package names, not file paths.
5555
*/
5656
const MAX_PATTERN_LENGTH = 4096
57+
// Cap wildcards to prevent backtracking DoS via many alternating `*` even
58+
// under the overall length limit (e.g., `a*b*c*…*z` on a long non-match).
59+
const MAX_WILDCARDS_PER_PATTERN = 32
60+
61+
function countWildcards(pattern: string): number {
62+
let count = 0
63+
for (let i = 0, { length } = pattern; i < length; i += 1) {
64+
const code = pattern.charCodeAt(i)
65+
if (code === 42 /*'*'*/ || code === 63 /*'?'*/) {
66+
count += 1
67+
}
68+
}
69+
return count
70+
}
5771

5872
function matchWildcard(pattern: string, value: string): boolean {
5973
// Reject excessively long patterns to prevent regex compilation DoS
6074
if (pattern.length > MAX_PATTERN_LENGTH) {
6175
return false
6276
}
77+
// Reject patterns with too many wildcards to prevent polynomial backtracking.
78+
if (countWildcards(pattern) > MAX_WILDCARDS_PER_PATTERN) {
79+
return false
80+
}
6381
let regex = wildcardRegexCache.get(pattern)
6482
if (regex === undefined) {
6583
// Convert glob pattern to regex

src/normalize.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,10 @@ function normalizeQualifiers(
9595
let qualifiers: Record<string, string> | undefined
9696
// Use for-of to work with entries iterators
9797
for (const { 0: key, 1: value } of qualifiersToEntries(rawQualifiers)) {
98+
// Skip non-string keys — validateQualifiers rejects these with PurlError.
99+
if (typeof key !== 'string') {
100+
continue
101+
}
98102
// Only coerce primitive types — reject objects/functions that could
99103
// execute arbitrary code via toString() during coercion.
100104
const strValue =

src/validate.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,12 @@ function validateQualifiers(
236236
// Use for-of to work with URLSearchParams#keys iterators
237237
// type-coverage:ignore-next-line -- TypeScript correctly infers the iteration type
238238
for (const key of keysIterable) {
239+
if (typeof key !== 'string') {
240+
if (throws) {
241+
throw new PurlError('qualifier key must be a string')
242+
}
243+
return false
244+
}
239245
if (!validateQualifierKey(key, opts)) {
240246
return false
241247
}

test/coverage-edge-cases.test.mts

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
findCommandInjectionCharCode,
1515
} from '../src/strings.js'
1616
import { encodeQualifiers } from '../src/encode.js'
17+
import { normalizeQualifiers } from '../src/normalize.js'
1718
import { stringify } from '../src/stringify.js'
1819
import { UrlConverter } from '../src/url-converter.js'
1920
import {
@@ -91,6 +92,28 @@ describe('validate edge cases', () => {
9192
})
9293
})
9394

95+
describe('validateQualifiers non-string key via custom keys iterator', () => {
96+
it('returns false for non-string key (non-throwing)', () => {
97+
const qualifiers = {
98+
keys() {
99+
return [42 as unknown as string][Symbol.iterator]()
100+
},
101+
}
102+
expect(validateQualifiers(qualifiers, { throws: false })).toBe(false)
103+
})
104+
105+
it('throws PurlError for non-string key (throwing)', () => {
106+
const qualifiers = {
107+
keys() {
108+
return [42 as unknown as string][Symbol.iterator]()
109+
},
110+
}
111+
expect(() => validateQualifiers(qualifiers, { throws: true })).toThrow(
112+
PurlError,
113+
)
114+
})
115+
})
116+
94117
describe('validateSubpath command injection', () => {
95118
it('returns false for subpath with pipe (non-throwing)', () => {
96119
expect(validateSubpath('src|evil', { throws: false })).toBe(false)
@@ -424,6 +447,21 @@ describe('compare edge cases', () => {
424447
)
425448
expect(matches(longPattern, purl)).toBe(false)
426449
})
450+
451+
it('returns false when pattern exceeds max wildcards (>32)', () => {
452+
// 33 wildcards in the name component — over MAX_WILDCARDS_PER_PATTERN (32)
453+
const manyWildcardsName = 'a*'.repeat(33)
454+
const pattern = `pkg:npm/${manyWildcardsName}`
455+
const purl = new PackageURL(
456+
'npm',
457+
undefined,
458+
'lodash',
459+
'4.17.21',
460+
undefined,
461+
undefined,
462+
)
463+
expect(matches(pattern, purl)).toBe(false)
464+
})
427465
})
428466

429467
describe('wildcard cache eviction', () => {
@@ -599,6 +637,71 @@ describe('encodeQualifiers edge case', () => {
599637
})
600638
})
601639

640+
// ---------------------------------------------------------------------------
641+
// normalize.ts — non-string key branch in normalizeQualifiers
642+
// ---------------------------------------------------------------------------
643+
describe('normalizeQualifiers non-string key handling', () => {
644+
it('skips non-string keys surfaced by a custom entries iterator', () => {
645+
const rawQualifiers = {
646+
entries() {
647+
return (
648+
[
649+
[42, 'ignored'],
650+
['real', 'kept'],
651+
] as unknown as Array<[string, string]>
652+
)[Symbol.iterator]()
653+
},
654+
}
655+
const result = normalizeQualifiers(rawQualifiers)
656+
expect(result).toEqual({ real: 'kept' })
657+
})
658+
})
659+
660+
// ---------------------------------------------------------------------------
661+
// purl-types/maven.ts — namespace must not contain a slash
662+
// ---------------------------------------------------------------------------
663+
describe('maven namespace slash validation', () => {
664+
it('returns false (non-throwing) when maven namespace contains a slash', () => {
665+
const validate = (PurlType as any).maven.validate as (
666+
purl: Record<string, unknown>,
667+
throws: boolean,
668+
) => boolean
669+
expect(
670+
validate(
671+
{
672+
type: 'maven',
673+
namespace: 'org.apache/commons',
674+
name: 'commons-lang3',
675+
version: '3.12.0',
676+
qualifiers: undefined,
677+
subpath: undefined,
678+
},
679+
false,
680+
),
681+
).toBe(false)
682+
})
683+
684+
it('throws PurlError when validating maven namespace with slash directly', () => {
685+
const validate = (PurlType as any).maven.validate as (
686+
purl: Record<string, unknown>,
687+
throws: boolean,
688+
) => boolean
689+
expect(() =>
690+
validate(
691+
{
692+
type: 'maven',
693+
namespace: 'org.apache/commons',
694+
name: 'commons-lang3',
695+
version: '3.12.0',
696+
qualifiers: undefined,
697+
subpath: undefined,
698+
},
699+
true,
700+
),
701+
).toThrow(PurlError)
702+
})
703+
})
704+
602705
// ---------------------------------------------------------------------------
603706
// package-url.ts — flyweight cache eviction in fromString
604707
// ---------------------------------------------------------------------------

0 commit comments

Comments
 (0)