Skip to content

Commit f48b5fa

Browse files
committed
prevent escaping symlinks with drive-relative paths
Fix: GHSA-9ppj-qmqm-q256
1 parent 97cff15 commit f48b5fa

File tree

3 files changed

+76
-3
lines changed

3 files changed

+76
-3
lines changed

src/unpack.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,7 @@ export class Unpack extends Parser {
303303
// tar paths, not a filesystem.
304304
const entryDir = path.posix.dirname(entry.path)
305305
const resolved = path.posix.normalize(
306-
path.posix.join(entryDir, p),
306+
path.posix.join(entryDir, parts.join('/'))
307307
)
308308
// If the resolved path escapes (starts with ..), reject it
309309
if (resolved.startsWith('../') || resolved === '..') {
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
/* IMPORTANT
2+
* This snapshot file is auto-generated, but designed for humans.
3+
* It should be checked into source control and tracked carefully.
4+
* Re-generate by setting TAP_SNAPSHOT=1 and running tests.
5+
* Make sure to inspect the output below. Do not ignore changes!
6+
*/
7+
'use strict'
8+
exports[`test/ghsa-8qq5-rm4j-mr97.ts > TAP > warnings > must match snapshot 1`] = `
9+
Array [
10+
Array [
11+
"TAR_ENTRY_INFO",
12+
"stripping / from absolute linkpath",
13+
],
14+
Array [
15+
"TAR_ENTRY_ERROR",
16+
"ENOENT: no such file or directory, link",
17+
],
18+
Array [
19+
"TAR_ENTRY_ERROR",
20+
"linkpath contains '..'",
21+
],
22+
Array [
23+
"TAR_ENTRY_INFO",
24+
"stripping / from absolute linkpath",
25+
],
26+
Array [
27+
"TAR_ENTRY_ERROR",
28+
"linkpath escapes extraction directory",
29+
],
30+
Array [
31+
"TAR_ENTRY_INFO",
32+
"stripping / from absolute linkpath",
33+
],
34+
Array [
35+
"TAR_ENTRY_INFO",
36+
"stripping c: from absolute linkpath",
37+
],
38+
Array [
39+
"TAR_ENTRY_ERROR",
40+
"linkpath escapes extraction directory",
41+
],
42+
]
43+
`

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

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,14 @@ import { Header, x } from 'tar'
1111
const targetSym = '/some/absolute/path'
1212
const absoluteWithDotDot = '/../a/target'
1313

14+
const secretLinkpath = resolve(t.testdirName, 'secret.txt')
15+
16+
t.formatSnapshot = (o: unknown): unknown =>
17+
typeof o === 'string' ? o.replace(
18+
/^ENOENT: no such file or directory, link .*? -> .*?$/, 'ENOENT: no such file or directory, link')
19+
: Array.isArray(o) ? o.map(o => t.formatSnapshot?.(o))
20+
: o
21+
1422
const getExploitTar = () => {
1523
const chunks: Buffer[] = []
1624

@@ -19,7 +27,7 @@ const getExploitTar = () => {
1927
path: 'exploit_hard',
2028
type: 'Link',
2129
size: 0,
22-
linkpath: resolve(t.testdirName, 'secret.txt'),
30+
linkpath: secretLinkpath,
2331
}).encode(hardHeader, 0)
2432
chunks.push(hardHeader)
2533

@@ -78,6 +86,14 @@ const getExploitTar = () => {
7886
}).encode(winAbsWithDotDotHeader, 0)
7987
chunks.push(winAbsWithDotDotHeader)
8088

89+
const winAbsWithDotDotEscapeHeader = Buffer.alloc(512)
90+
new Header({
91+
path: 'a/winrootdotsescapelink',
92+
type: 'SymbolicLink',
93+
linkpath: 'c:..\\..\\..\\..\\foo\\bar',
94+
}).encode(winAbsWithDotDotEscapeHeader, 0)
95+
chunks.push(winAbsWithDotDotEscapeHeader)
96+
8197
chunks.push(Buffer.alloc(1024))
8298
return Buffer.concat(chunks)
8399
}
@@ -91,7 +107,17 @@ const dir = t.testdir({
91107
const out = resolve(dir, 'out')
92108
const tarFile = resolve(dir, 'exploit.tar')
93109

94-
t.before(() => x({ cwd: out, file: tarFile }))
110+
const WARNINGS: [code: string, message: string][] = []
111+
t.before(() =>
112+
x({
113+
cwd: out,
114+
file: tarFile,
115+
onwarn: (code: string, message: string) =>
116+
WARNINGS.push([code, message]),
117+
}),
118+
)
119+
120+
t.test('warnings', async t => t.matchSnapshot(WARNINGS))
95121

96122
t.test('writefile exploits fail', async t => {
97123
writeFileSync(resolve(out, 'exploit_hard'), 'OVERWRITTEN')
@@ -126,4 +152,8 @@ t.test('absolute symlink with .. has prefix stripped', async t => {
126152
'..\\foo\\bar',
127153
'symlink target should be normalized',
128154
)
155+
t.throws(
156+
() => lstatSync(resolve(out, 'a/winrootdotsescapelink')),
157+
'escaping symlink is not created',
158+
)
129159
})

0 commit comments

Comments
 (0)