Skip to content

Commit ce1c013

Browse files
fengmk2fengmk2
authored
Merge commit from fork
Add path traversal and symlink escape protection to prevent malicious TAR/TGZ archives from writing files outside the extraction directory. - Add isPathWithinParent() validation function - Validate all entry paths stay within destination directory - Validate symlink targets don't escape extraction directory - Skip malicious entries with warning messages
1 parent fd321da commit ce1c013

File tree

2 files changed

+267
-1
lines changed

2 files changed

+267
-1
lines changed

‎lib/utils.js‎

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,22 @@ const fs = require('fs');
44
const path = require('path');
55
const { pipeline: pump } = require('stream');
66

7+
/**
8+
* Check if childPath is within parentPath (prevents path traversal attacks)
9+
* @param {string} childPath - The path to check
10+
* @param {string} parentPath - The parent directory path
11+
* @returns {boolean} - True if childPath is within parentPath
12+
*/
13+
function isPathWithinParent(childPath, parentPath) {
14+
const normalizedChild = path.resolve(childPath);
15+
const normalizedParent = path.resolve(parentPath);
16+
const parentWithSep = normalizedParent.endsWith(path.sep)
17+
? normalizedParent
18+
: normalizedParent + path.sep;
19+
return normalizedChild === normalizedParent ||
20+
normalizedChild.startsWith(parentWithSep);
21+
}
22+
723
// file/fileBuffer/stream
824
exports.sourceType = source => {
925
if (!source) return undefined;
@@ -92,6 +108,9 @@ exports.makeUncompressFn = StreamClass => {
92108
fs.mkdir(destDir, { recursive: true }, err => {
93109
if (err) return reject(err);
94110

111+
// Resolve destDir to absolute path for security validation
112+
const resolvedDestDir = path.resolve(destDir);
113+
95114
let entryCount = 0;
96115
let successCount = 0;
97116
let isFinish = false;
@@ -108,7 +127,15 @@ exports.makeUncompressFn = StreamClass => {
108127
.on('error', reject)
109128
.on('entry', (header, stream, next) => {
110129
stream.on('end', next);
111-
const destFilePath = path.join(destDir, header.name);
130+
const destFilePath = path.join(resolvedDestDir, header.name);
131+
const resolvedDestPath = path.resolve(destFilePath);
132+
133+
// Security: Validate that the entry path doesn't escape the destination directory
134+
if (!isPathWithinParent(resolvedDestPath, resolvedDestDir)) {
135+
console.warn(`[compressing] Skipping entry with path traversal: "${header.name}" -> "${resolvedDestPath}"`);
136+
stream.resume();
137+
return;
138+
}
112139

113140
if (header.type === 'file') {
114141
const dir = path.dirname(destFilePath);
@@ -125,6 +152,14 @@ exports.makeUncompressFn = StreamClass => {
125152
} else if (header.type === 'symlink') {
126153
const dir = path.dirname(destFilePath);
127154
const target = path.resolve(dir, header.linkname);
155+
156+
// Security: Validate that the symlink target doesn't escape the destination directory
157+
if (!isPathWithinParent(target, resolvedDestDir)) {
158+
console.warn(`[compressing] Skipping symlink "${header.name}": target "${target}" escapes extraction directory`);
159+
stream.resume();
160+
return;
161+
}
162+
128163
entryCount++;
129164

130165
fs.mkdir(dir, { recursive: true }, err => {

‎test/tar/security-GHSA-cc8f-xg8v-72m3.test.js‎

Lines changed: 231 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,231 @@
1+
'use strict';
2+
3+
const fs = require('fs');
4+
const os = require('os');
5+
const path = require('path');
6+
const uuid = require('uuid');
7+
const assert = require('assert');
8+
const tar = require('tar-stream');
9+
const compressing = require('../..');
10+
11+
describe('test/tar/security-GHSA-cc8f-xg8v-72m3.test.js', () => {
12+
let tempDir;
13+
14+
beforeEach(() => {
15+
tempDir = path.join(os.tmpdir(), uuid.v4());
16+
fs.mkdirSync(tempDir, { recursive: true });
17+
});
18+
19+
afterEach(() => {
20+
fs.rmSync(tempDir, { recursive: true, force: true });
21+
});
22+
23+
/**
24+
* Helper function to create a TAR buffer with given entries
25+
* @param {Array<{name: string, type?: string, linkname?: string, content?: string}>} entries
26+
* @returns {Promise<Buffer>}
27+
*/
28+
function createTarBuffer(entries) {
29+
return new Promise((resolve, reject) => {
30+
const pack = tar.pack();
31+
const chunks = [];
32+
33+
pack.on('data', chunk => chunks.push(chunk));
34+
pack.on('end', () => resolve(Buffer.concat(chunks)));
35+
pack.on('error', reject);
36+
37+
for (const entry of entries) {
38+
if (entry.type === 'symlink') {
39+
pack.entry({ name: entry.name, type: 'symlink', linkname: entry.linkname });
40+
} else if (entry.type === 'directory') {
41+
pack.entry({ name: entry.name, type: 'directory' });
42+
} else {
43+
pack.entry({ name: entry.name, type: 'file' }, entry.content || '');
44+
}
45+
}
46+
47+
pack.finalize();
48+
});
49+
}
50+
51+
describe('symlink escape vulnerability (CVE-2021-32803 style)', () => {
52+
it('should block symlink pointing outside extraction directory', async () => {
53+
const destDir = path.join(tempDir, 'dest');
54+
const escapedFile = path.join(tempDir, 'escaped.txt');
55+
56+
// Create malicious TAR:
57+
// 1. Symlink "escape" -> ".." (points to parent of dest)
58+
// 2. File "escape/escaped.txt" (would write to tempDir/escaped.txt if symlink was followed)
59+
const tarBuffer = await createTarBuffer([
60+
{ name: 'escape', type: 'symlink', linkname: '..' },
61+
{ name: 'escape/escaped.txt', type: 'file', content: 'malicious content' },
62+
]);
63+
64+
await compressing.tar.uncompress(tarBuffer, destDir);
65+
66+
// The escaped file should NOT exist in the parent directory
67+
assert.strictEqual(fs.existsSync(escapedFile), false, 'File should not be written outside destination');
68+
69+
// The symlink should NOT exist as a symlink (it may exist as a directory now)
70+
const escapePath = path.join(destDir, 'escape');
71+
if (fs.existsSync(escapePath)) {
72+
const stat = fs.lstatSync(escapePath);
73+
assert.strictEqual(stat.isSymbolicLink(), false, 'Path should not be a symlink');
74+
}
75+
});
76+
77+
it('should block symlink pointing to absolute path outside extraction directory', async () => {
78+
const destDir = path.join(tempDir, 'dest');
79+
const escapedFile = path.join(tempDir, 'poc.txt');
80+
81+
// Create malicious TAR with symlink pointing to absolute path
82+
const tarBuffer = await createTarBuffer([
83+
{ name: 'myTmp', type: 'symlink', linkname: tempDir },
84+
{ name: 'myTmp/poc.txt', type: 'file', content: 'malicious content' },
85+
]);
86+
87+
await compressing.tar.uncompress(tarBuffer, destDir);
88+
89+
// The escaped file should NOT exist
90+
assert.strictEqual(fs.existsSync(escapedFile), false, 'File should not be written via symlink escape');
91+
});
92+
93+
it('should block symlink with absolute path target like /etc/passwd', async () => {
94+
const destDir = path.join(tempDir, 'dest');
95+
96+
// Create malicious TAR with symlink pointing to /etc/passwd
97+
const tarBuffer = await createTarBuffer([
98+
{ name: 'passwd', type: 'symlink', linkname: '/etc/passwd' },
99+
]);
100+
101+
await compressing.tar.uncompress(tarBuffer, destDir);
102+
103+
// The symlink should NOT be created
104+
assert.strictEqual(fs.existsSync(path.join(destDir, 'passwd')), false, 'Symlink to /etc/passwd should not be created');
105+
});
106+
});
107+
108+
describe('path traversal via file entries', () => {
109+
it('should block file entry with ../ path traversal', async () => {
110+
const destDir = path.join(tempDir, 'dest');
111+
const escapedFile = path.join(tempDir, 'traversed.txt');
112+
113+
// Create malicious TAR with path traversal
114+
const tarBuffer = await createTarBuffer([
115+
{ name: '../traversed.txt', type: 'file', content: 'malicious' },
116+
]);
117+
118+
await compressing.tar.uncompress(tarBuffer, destDir);
119+
120+
// The file should NOT exist outside destination
121+
assert.strictEqual(fs.existsSync(escapedFile), false, 'File should not be written via path traversal');
122+
});
123+
124+
it('should block file entry with nested ../ path traversal', async () => {
125+
const destDir = path.join(tempDir, 'dest');
126+
const escapedFile = path.join(tempDir, 'nested-escape.txt');
127+
128+
// Create malicious TAR with nested path traversal
129+
const tarBuffer = await createTarBuffer([
130+
{ name: 'foo/bar/../../nested-escape.txt', type: 'file', content: 'malicious' },
131+
]);
132+
133+
await compressing.tar.uncompress(tarBuffer, destDir);
134+
135+
// The file should NOT exist outside destination
136+
// (Note: The file might be written as dest/nested-escape.txt after normalization, which is acceptable)
137+
assert.strictEqual(fs.existsSync(escapedFile), false, 'File should not escape to parent via nested traversal');
138+
});
139+
140+
it('should block directory entry with ../ path traversal', async () => {
141+
const destDir = path.join(tempDir, 'dest');
142+
const escapedDir = path.join(tempDir, 'escaped-dir');
143+
144+
// Create malicious TAR with directory path traversal
145+
const tarBuffer = await createTarBuffer([
146+
{ name: '../escaped-dir', type: 'directory' },
147+
]);
148+
149+
await compressing.tar.uncompress(tarBuffer, destDir);
150+
151+
// The directory should NOT exist outside destination
152+
assert.strictEqual(fs.existsSync(escapedDir), false, 'Directory should not be created via path traversal');
153+
});
154+
});
155+
156+
describe('backward compatibility - valid symlinks', () => {
157+
it('should allow valid internal symlinks', async () => {
158+
const destDir = path.join(tempDir, 'dest');
159+
160+
// Create TAR with valid internal symlink
161+
const tarBuffer = await createTarBuffer([
162+
{ name: 'real-file.txt', type: 'file', content: 'hello world' },
163+
{ name: 'link-to-file.txt', type: 'symlink', linkname: 'real-file.txt' },
164+
]);
165+
166+
await compressing.tar.uncompress(tarBuffer, destDir);
167+
168+
// Both files should exist
169+
assert.strictEqual(fs.existsSync(path.join(destDir, 'real-file.txt')), true, 'Real file should exist');
170+
assert.strictEqual(fs.existsSync(path.join(destDir, 'link-to-file.txt')), true, 'Symlink should exist');
171+
172+
// Symlink should point to the real file
173+
const linkTarget = fs.readlinkSync(path.join(destDir, 'link-to-file.txt'));
174+
assert.strictEqual(linkTarget, 'real-file.txt', 'Symlink should point to real-file.txt');
175+
});
176+
177+
it('should allow symlinks within subdirectories', async () => {
178+
const destDir = path.join(tempDir, 'dest');
179+
180+
// Create TAR with valid symlink in subdirectory
181+
const tarBuffer = await createTarBuffer([
182+
{ name: 'subdir/', type: 'directory' },
183+
{ name: 'subdir/file.txt', type: 'file', content: 'content' },
184+
{ name: 'subdir/link.txt', type: 'symlink', linkname: 'file.txt' },
185+
]);
186+
187+
await compressing.tar.uncompress(tarBuffer, destDir);
188+
189+
// Both should exist
190+
assert.strictEqual(fs.existsSync(path.join(destDir, 'subdir/file.txt')), true);
191+
assert.strictEqual(fs.existsSync(path.join(destDir, 'subdir/link.txt')), true);
192+
});
193+
194+
it('should extract symlink.tgz fixture correctly', async () => {
195+
const sourceFile = path.join(__dirname, '..', 'fixtures', 'symlink.tgz');
196+
const destDir = path.join(tempDir, 'symlink-test');
197+
198+
// This should not throw
199+
await compressing.tgz.uncompress(sourceFile, destDir);
200+
201+
// Verify destination was created
202+
assert.strictEqual(fs.existsSync(destDir), true, 'Destination directory should exist');
203+
});
204+
});
205+
206+
describe('edge cases', () => {
207+
it('should handle empty TAR', async () => {
208+
const destDir = path.join(tempDir, 'dest');
209+
const tarBuffer = await createTarBuffer([]);
210+
211+
await compressing.tar.uncompress(tarBuffer, destDir);
212+
213+
assert.strictEqual(fs.existsSync(destDir), true, 'Destination should be created');
214+
});
215+
216+
it('should handle normal files correctly', async () => {
217+
const destDir = path.join(tempDir, 'dest');
218+
219+
const tarBuffer = await createTarBuffer([
220+
{ name: 'file1.txt', type: 'file', content: 'content1' },
221+
{ name: 'subdir/', type: 'directory' },
222+
{ name: 'subdir/file2.txt', type: 'file', content: 'content2' },
223+
]);
224+
225+
await compressing.tar.uncompress(tarBuffer, destDir);
226+
227+
assert.strictEqual(fs.readFileSync(path.join(destDir, 'file1.txt'), 'utf8'), 'content1');
228+
assert.strictEqual(fs.readFileSync(path.join(destDir, 'subdir/file2.txt'), 'utf8'), 'content2');
229+
});
230+
});
231+
});

0 commit comments

Comments
 (0)