Skip to content

Commit 310dcbf

Browse files
authored
fix: reject path traversal sequences in extras validator (#482)
The extras validator was a no-op and accepted any value. Reject values containing parent directory references (..), backslashes, encoded slashes (%2F) or backslashes (%5C), and protocol-relative prefixes (//), including multi-level percent-encoded variants.
1 parent dcbfd54 commit 310dcbf

2 files changed

Lines changed: 73 additions & 4 deletions

File tree

‎lib/validators/extra.js‎

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,41 @@
11
/**
2-
* No-op, returning its value.
2+
* Validates a path extras value, rejecting sequences that could be used
3+
* to escape the expected package path via encoded characters.
4+
*
5+
* Rejects: ".." segments, backslashes (literal or encoded), encoded slashes,
6+
* and protocol-relative "//" prefixes.
7+
*
38
* @param {string} value
49
* @returns {string} the value
5-
* @todo https://github.com/asset-pipe/core/issues/12
6-
* @todo Can we remove this?
710
*/
8-
export const extra = (value) => value;
11+
export const extra = (value) => {
12+
if (!value) return value;
13+
14+
// Fully decode to catch multi-level percent-encoding (%252F → %2F → /).
15+
// Run twice to handle double-encoding; stable after two passes for all
16+
// realistic inputs.
17+
let once;
18+
let twice;
19+
try {
20+
once = decodeURIComponent(value);
21+
twice = decodeURIComponent(once);
22+
} catch {
23+
throw new Error('Parameter "extra" is not valid');
24+
}
25+
26+
if (
27+
twice.includes("..") ||
28+
twice.includes("\\") ||
29+
twice.includes("//") ||
30+
// Reject any remaining encoded slashes or backslashes that survived
31+
// after decoding (indicates triple+ encoding or other evasion).
32+
twice.includes("%2F") ||
33+
twice.includes("%2f") ||
34+
twice.includes("%5C") ||
35+
twice.includes("%5c")
36+
) {
37+
throw new Error('Parameter "extra" is not valid');
38+
}
39+
40+
return value;
41+
};

‎test/validators/index.test.js‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,42 @@ test(".extra() - valid values - should return value", () => {
210210
assert.strictEqual(validators.extra("index.js"), "index.js");
211211
});
212212

213+
test(".extra() - path traversal with .. - should throw", () => {
214+
assert.throws(() => {
215+
validators.extra("../../../etc/passwd");
216+
}, /Parameter "extra" is not valid/);
217+
});
218+
219+
test(".extra() - encoded path traversal %2F - should throw", () => {
220+
assert.throws(() => {
221+
validators.extra("..%2F..%2F..%2Fetc%2Fpasswd");
222+
}, /Parameter "extra" is not valid/);
223+
});
224+
225+
test(".extra() - double-encoded path traversal %252F - should throw", () => {
226+
assert.throws(() => {
227+
validators.extra("..%252F..%252F..%252F..%252F%255Crmssport.dk");
228+
}, /Parameter "extra" is not valid/);
229+
});
230+
231+
test(".extra() - backslash - should throw", () => {
232+
assert.throws(() => {
233+
validators.extra("..\\rmssport.dk");
234+
}, /Parameter "extra" is not valid/);
235+
});
236+
237+
test(".extra() - encoded backslash %5C - should throw", () => {
238+
assert.throws(() => {
239+
validators.extra("..%5Crmssport.dk");
240+
}, /Parameter "extra" is not valid/);
241+
});
242+
243+
test(".extra() - protocol-relative // - should throw", () => {
244+
assert.throws(() => {
245+
validators.extra("//rmssport.dk");
246+
}, /Parameter "extra" is not valid/);
247+
});
248+
213249
//
214250
// .semverType()
215251
//

0 commit comments

Comments
 (0)