Skip to content

Commit f4a7aa9

Browse files
committed
fix: properly sanitize hard links containing ..
Fix: GHSA-34x7-hfp2-rc4v The issue here is that *hard* links are resolved relative to the unpack cwd, so if they have `..`, they cannot possibly be valid, same as files and for the same reason. The loosening of this restriction for symbolic links should have been limited by type, allowing this error.
1 parent 394ece6 commit f4a7aa9

File tree

2 files changed

+22
-1
lines changed

2 files changed

+22
-1
lines changed

src/unpack.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,7 @@ export class Unpack extends Parser {
271271
field: 'path' | 'linkpath',
272272
): boolean {
273273
const p = entry[field]
274+
const { type } = entry
274275
if (!p || this.preservePaths) return true
275276

276277
const parts = p.split('/')
@@ -284,7 +285,7 @@ export class Unpack extends Parser {
284285
// just rejecting any path with '..' - relative symlinks like
285286
// '../sibling/file' are valid if they resolve within the cwd.
286287
// For paths, they just simply may not ever use .. at all.
287-
if (field === 'path') {
288+
if (field === 'path' || type === 'Link') {
288289
this.warn('TAR_ENTRY_ERROR', `${field} contains '..'`, {
289290
entry,
290291
[field]: p,

test/ghsa-8qq5-rm4j-mr97.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,20 @@ const getExploitTar = () => {
2323
}).encode(hardHeader, 0)
2424
chunks.push(hardHeader)
2525

26+
const hardSubHeader = Buffer.alloc(1024)
27+
new Header({
28+
path: 'sub/',
29+
type: 'Directory',
30+
size: 0,
31+
}).encode(hardSubHeader, 0)
32+
new Header({
33+
path: 'sub/exploit_sub',
34+
type: 'Link',
35+
size: 0,
36+
linkpath: '../secret.txt',
37+
}).encode(hardSubHeader, 512)
38+
chunks.push(hardSubHeader)
39+
2640
const symHeader = Buffer.alloc(512)
2741
new Header({
2842
path: 'exploit_sym',
@@ -77,6 +91,12 @@ t.test('hardlink escape does not clobber target', async t => {
7791
readFileSync(resolve(dir, 'secret.txt'), 'utf8'),
7892
'ORIGINAL DATA',
7993
)
94+
95+
writeFileSync(resolve(out, 'sub/exploit_sub'), 'OVERWRITTEN SUB')
96+
t.equal(
97+
readFileSync(resolve(dir, 'secret.txt'), 'utf8'),
98+
'ORIGINAL DATA',
99+
)
80100
})
81101

82102
t.test('symlink escapes are sanitized', async t => {

0 commit comments

Comments
 (0)