Skip to content

Commit 7bc755d

Browse files
committed
parse root off paths before sanitizing .. parts
Fix: GHSA-qffp-2rhf-9h96
1 parent c8cb846 commit 7bc755d

File tree

3 files changed

+20
-10
lines changed

3 files changed

+20
-10
lines changed

src/strip-absolute-path.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ const { isAbsolute, parse } = win32
88
// explicitly if it's the first character.
99
// drive-specific relative paths on Windows get their root stripped off even
1010
// though they are not absolute, so `c:../foo` becomes ['c:', '../foo']
11-
export const stripAbsolutePath = (path: string) => {
11+
export const stripAbsolutePath = (path: string): [string, string] => {
1212
let r = ''
1313

1414
let parsed = parse(path)

src/unpack.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,9 @@ export class Unpack extends Parser {
277277
const { type } = entry
278278
if (!p || this.preservePaths) return true
279279

280-
const parts = p.split('/')
280+
// strip off the root
281+
const [root, stripped] = stripAbsolutePath(p)
282+
const parts = stripped.replace(/\\/g, '/').split('/')
281283

282284
if (
283285
parts.includes('..') ||
@@ -318,8 +320,6 @@ export class Unpack extends Parser {
318320
}
319321
}
320322

321-
// strip off the root
322-
const [root, stripped] = stripAbsolutePath(p)
323323
if (root) {
324324
// ok, but triggers warning about stripping root
325325
entry[field] = String(stripped)

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

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,14 @@ const getExploitTar = () => {
7070
}).encode(absWithDotDotHeader, 0)
7171
chunks.push(absWithDotDotHeader)
7272

73+
const winAbsWithDotDotHeader = Buffer.alloc(512)
74+
new Header({
75+
path: 'a/winrootdotslink',
76+
type: 'SymbolicLink',
77+
linkpath: 'c:..\\foo\\bar',
78+
}).encode(winAbsWithDotDotHeader, 0)
79+
chunks.push(winAbsWithDotDotHeader)
80+
7381
chunks.push(Buffer.alloc(1024))
7482
return Buffer.concat(chunks)
7583
}
@@ -83,9 +91,9 @@ const dir = t.testdir({
8391
const out = resolve(dir, 'out')
8492
const tarFile = resolve(dir, 'exploit.tar')
8593

86-
t.test('hardlink escape does not clobber target', async t => {
87-
await x({ cwd: out, file: tarFile })
94+
t.before(() => x({ cwd: out, file: tarFile }))
8895

96+
t.test('writefile exploits fail', async t => {
8997
writeFileSync(resolve(out, 'exploit_hard'), 'OVERWRITTEN')
9098
t.equal(
9199
readFileSync(resolve(dir, 'secret.txt'), 'utf8'),
@@ -100,20 +108,22 @@ t.test('hardlink escape does not clobber target', async t => {
100108
})
101109

102110
t.test('symlink escapes are sanitized', async t => {
103-
await x({ cwd: out, file: tarFile })
104-
105111
t.not(readlinkSync(resolve(out, 'exploit_sym')), targetSym)
106112
t.throws(() => lstatSync(resolve(out, 'secret.txt')), {
107113
code: 'ENOENT',
108114
})
109115
})
110116

111117
t.test('absolute symlink with .. has prefix stripped', async t => {
112-
await x({ cwd: out, file: tarFile })
113-
114118
t.equal(
115119
readlinkSync(resolve(out, 'a/link')),
116120
'../a/target',
117121
'symlink target should be normalized',
118122
)
123+
124+
t.equal(
125+
readlinkSync(resolve(out, 'a/winrootdotslink')),
126+
'..\\foo\\bar',
127+
'symlink target should be normalized',
128+
)
119129
})

0 commit comments

Comments
 (0)