GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Merge commit from fork · node-modules/compressing@0495f37

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
  • lib
    • utils.js
  • test/tar
    • security-GHSA-grx6-75c5-f738.test.js
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎lib/utils.js‎

Copy file name to clipboardExpand all lines: lib/utils.js
+28-5Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,23 @@ async function unlinkSymlink(target) {
3131
}
3232
}
3333

34+
/**
35+
* Resolve existing ancestors without creating missing directories.
36+
* @param {string} dir - Absolute directory path to resolve
37+
* @return {Promise<string>} Path with existing ancestors resolved
38+
*/
39+
async function realpathWithMissingDirectories(dir) {
40+
try {
41+
// Do not treat an existing dangling symlink as a missing directory.
42+
await fs.promises.lstat(dir);
43+
} catch (err) {
44+
const parent = path.dirname(dir);
45+
if (err.code !== 'ENOENT' || parent === dir) throw err;
46+
return path.join(await realpathWithMissingDirectories(parent), path.basename(dir));
47+
}
48+
return fs.promises.realpath(dir);
49+
}
50+
3451
/**
3552
* Check if childPath is within parentPath (prevents path traversal attacks)
3653
* @param {string} childPath - The path to check
@@ -257,9 +274,15 @@ exports.makeUncompressFn = StreamClass => {
257274
});
258275
} else if (header.type === 'symlink') {
259276
const dir = path.dirname(destFilePath);
260-
const target = path.resolve(dir, header.linkname);
261-
262-
if (!isPathWithinParent(target, resolvedDestDir)) {
277+
// Relative link targets are interpreted from the real parent directory.
278+
// An internal symlink such as sub/d -> .. can collapse the textual depth.
279+
const realDir = await realpathWithMissingDirectories(dir);
280+
let target = path.resolve(realDir, header.linkname);
281+
282+
if (isPathWithinParent(target, resolvedDestDir)) {
283+
// Map the extraction root's alias without resolving a possibly missing target.
284+
target = path.join(realDestDir, path.relative(resolvedDestDir, target));
285+
} else if (!isPathWithinParent(target, realDestDir)) {
263286
console.warn(`[compressing] Skipping symlink "${header.name}": target "${target}" escapes extraction directory`);
264287
stream.resume();
265288
return;
@@ -271,9 +294,9 @@ exports.makeUncompressFn = StreamClass => {
271294
return;
272295
}
273296

274-
entryCount++;
275297
await fs.promises.mkdir(dir, { recursive: true });
276-
const relativeTarget = path.relative(dir, target);
298+
entryCount++;
299+
const relativeTarget = path.relative(realDir, target) || '.';
277300
await fs.promises.symlink(relativeTarget, destFilePath);
278301
successCount++;
279302
stream.resume();
Collapse file

‎test/tar/security-GHSA-grx6-75c5-f738.test.js‎

Copy file name to clipboard
+234Lines changed: 234 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,234 @@
1+
'use strict';
2+
3+
const assert = require('assert');
4+
const fs = require('fs');
5+
const os = require('os');
6+
const path = require('path');
7+
const zlib = require('zlib');
8+
const compressing = require('../..');
9+
const { createTarBuffer } = require('../util');
10+
11+
describe('test/tar/security-GHSA-grx6-75c5-f738.test.js', () => {
12+
let tempDir;
13+
14+
beforeEach(() => {
15+
tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'compressing-symlink-'));
16+
});
17+
18+
afterEach(() => {
19+
fs.rmSync(tempDir, { recursive: true, force: true });
20+
});
21+
22+
[ 'tar', 'tgz' ].forEach(format => {
23+
describe(format, () => {
24+
async function extract(entries, destDir, opts) {
25+
const tarBuffer = await createTarBuffer(entries);
26+
const archive = format === 'tgz' ? zlib.gzipSync(tarBuffer) : tarBuffer;
27+
await compressing[format].uncompress(archive, destDir, opts);
28+
}
29+
30+
[ 'existing target', 'dangling target', 'strip' ].forEach(scenario => {
31+
it(`should block a symlink escape through repeated directory links (${scenario})`, async () => {
32+
const targetExists = scenario !== 'dangling target';
33+
const strip = scenario === 'strip' ? 1 : 0;
34+
const prefix = strip ? 'package/' : '';
35+
36+
// Eight textual parent segments collapse to the extraction root. Keep
37+
// the actual target inside tempDir, eight levels above that root.
38+
const destDir = path.join(tempDir, ...Array(8).fill('dest'));
39+
const outsideFile = path.join(tempDir, 'outside.txt');
40+
if (targetExists) fs.writeFileSync(outsideFile, 'original');
41+
42+
await extract([
43+
{ name: `${prefix}sub/`, type: 'directory' },
44+
{ name: `${prefix}sub/d`, type: 'symlink', linkname: '..' },
45+
{ name: `${prefix}${'sub/d/'.repeat(4)}link`, type: 'symlink', linkname: `${'../'.repeat(8)}outside.txt` },
46+
{ name: `${prefix}after.txt`, content: 'safe' },
47+
], destDir, { strip });
48+
49+
assert.strictEqual(fs.realpathSync(path.join(destDir, 'sub/d')), fs.realpathSync(destDir));
50+
// existsSync() also returns false for dangling links, so use lstat().
51+
assert.throws(() => fs.lstatSync(path.join(destDir, 'link')), { code: 'ENOENT' });
52+
assert.strictEqual(fs.readFileSync(path.join(destDir, 'after.txt'), 'utf8'), 'safe');
53+
if (targetExists) {
54+
assert.strictEqual(fs.readFileSync(outsideFile, 'utf8'), 'original');
55+
} else {
56+
assert.strictEqual(fs.existsSync(outsideFile), false);
57+
}
58+
});
59+
});
60+
61+
it('should still block a target that escapes the textual parent', async () => {
62+
const destDir = path.join(tempDir, 'dest');
63+
await extract([
64+
{ name: 'sub/', type: 'directory' },
65+
{ name: 'sub/d', type: 'symlink', linkname: '..' },
66+
{ name: `${'sub/d/'.repeat(4)}link`, type: 'symlink', linkname: `${'../'.repeat(9)}outside.txt` },
67+
], destDir);
68+
69+
assert.throws(() => fs.lstatSync(path.join(destDir, 'link')), { code: 'ENOENT' });
70+
});
71+
72+
it('should normalize directory-link traversal in the symlink target', async () => {
73+
const destDir = path.join(tempDir, ...Array(8).fill('dest'));
74+
const outsideFile = path.join(tempDir, 'target.txt');
75+
fs.writeFileSync(outsideFile, 'outside');
76+
77+
await extract([
78+
{ name: 'sub/', type: 'directory' },
79+
{ name: 'sub/d', type: 'symlink', linkname: '..' },
80+
{ name: 'link', type: 'symlink', linkname: `${'sub/d/'.repeat(4)}${'../'.repeat(8)}target.txt` },
81+
], destDir);
82+
83+
const link = path.join(destDir, 'link');
84+
// The raw target would follow sub/d back to destDir before walking up.
85+
assert.strictEqual(fs.readlinkSync(link), 'target.txt');
86+
assert.throws(() => fs.realpathSync(link), { code: 'ENOENT' });
87+
fs.writeFileSync(path.join(destDir, 'target.txt'), 'inside');
88+
assert.strictEqual(fs.readFileSync(link, 'utf8'), 'inside');
89+
assert.strictEqual(fs.readFileSync(outsideFile, 'utf8'), 'outside');
90+
});
91+
92+
it('should block a symlink target that still escapes after normalization', async () => {
93+
const destDir = path.join(tempDir, ...Array(8).fill('dest'));
94+
const outsideFile = path.join(tempDir, 'target.txt');
95+
fs.writeFileSync(outsideFile, 'outside');
96+
97+
await extract([
98+
{ name: 'sub/', type: 'directory' },
99+
{ name: 'sub/d', type: 'symlink', linkname: '..' },
100+
{ name: 'link', type: 'symlink', linkname: `${'sub/d/'.repeat(2)}${'../'.repeat(8)}target.txt` },
101+
{ name: 'after.txt', content: 'safe' },
102+
], destDir);
103+
104+
assert.throws(() => fs.lstatSync(path.join(destDir, 'link')), { code: 'ENOENT' });
105+
assert.strictEqual(fs.readFileSync(path.join(destDir, 'after.txt'), 'utf8'), 'safe');
106+
assert.strictEqual(fs.readFileSync(outsideFile, 'utf8'), 'outside');
107+
});
108+
109+
[ 'relative', 'absolute', 'external symlink' ].forEach(scenario => {
110+
it(`should extract a file at the parent path of a rejected symlink (${scenario})`, async () => {
111+
const destDir = path.join(tempDir, 'dest');
112+
const outsideDir = path.join(tempDir, 'outside');
113+
fs.mkdirSync(outsideDir);
114+
fs.writeFileSync(path.join(outsideDir, 'target.txt'), 'original');
115+
116+
let linkname = '../../../outside/target.txt';
117+
if (scenario === 'absolute') {
118+
linkname = path.join(outsideDir, 'target.txt');
119+
} else if (scenario === 'external symlink') {
120+
fs.mkdirSync(destDir);
121+
fs.symlinkSync(outsideDir, path.join(destDir, 'escape'), 'dir');
122+
linkname = '../../escape/target.txt';
123+
}
124+
125+
await extract([
126+
{ name: 'payload/nested/link', type: 'symlink', linkname },
127+
{ name: 'payload', content: 'safe' },
128+
{ name: 'after.txt', content: 'safe' },
129+
], destDir);
130+
131+
assert.strictEqual(fs.lstatSync(path.join(destDir, 'payload')).isFile(), true);
132+
assert.strictEqual(fs.readFileSync(path.join(destDir, 'payload'), 'utf8'), 'safe');
133+
assert.strictEqual(fs.readFileSync(path.join(destDir, 'after.txt'), 'utf8'), 'safe');
134+
assert.strictEqual(fs.readFileSync(path.join(outsideDir, 'target.txt'), 'utf8'), 'original');
135+
});
136+
});
137+
138+
it('should check the real parent when intermediate directories must be created', async () => {
139+
const destDir = path.join(tempDir, 'dest');
140+
await extract([
141+
{ name: 'sub/', type: 'directory' },
142+
{ name: 'sub/d', type: 'symlink', linkname: '..' },
143+
{ name: 'sub/d/new/link', type: 'symlink', linkname: '../../outside.txt' },
144+
], destDir);
145+
146+
assert.throws(() => fs.lstatSync(path.join(destDir, 'new')), { code: 'ENOENT' });
147+
});
148+
149+
it('should allow a safe relative target through directory links', async () => {
150+
const destDir = path.join(tempDir, 'dest');
151+
await extract([
152+
{ name: 'sub/', type: 'directory' },
153+
{ name: 'sub/d', type: 'symlink', linkname: '..' },
154+
{ name: 'sub/d/sub/d/new/link', type: 'symlink', linkname: '../target.txt' },
155+
{ name: 'sub/d/sub/d/new/self', type: 'symlink', linkname: '.' },
156+
{ name: 'target.txt', content: 'safe' },
157+
], destDir);
158+
159+
const link = path.join(destDir, 'new/link');
160+
assert.strictEqual(fs.lstatSync(link).isSymbolicLink(), true);
161+
assert.strictEqual(fs.realpathSync(link), fs.realpathSync(path.join(destDir, 'target.txt')));
162+
assert.strictEqual(fs.readFileSync(link, 'utf8'), 'safe');
163+
assert.strictEqual(fs.realpathSync(path.join(destDir, 'new/self')), fs.realpathSync(path.join(destDir, 'new')));
164+
});
165+
166+
it('should block an internal target that resolves through an external symlink', async () => {
167+
const destDir = path.join(tempDir, 'dest');
168+
const outsideDir = path.join(tempDir, 'outside');
169+
fs.mkdirSync(destDir);
170+
fs.mkdirSync(outsideDir);
171+
fs.writeFileSync(path.join(outsideDir, 'target.txt'), 'original');
172+
fs.symlinkSync(outsideDir, path.join(destDir, 'outside'), 'dir');
173+
174+
await extract([
175+
{ name: 'sub/', type: 'directory' },
176+
{ name: 'sub/d', type: 'symlink', linkname: '..' },
177+
{ name: 'sub/d/sub/d/link', type: 'symlink', linkname: 'outside/target.txt' },
178+
], destDir);
179+
180+
assert.throws(() => fs.lstatSync(path.join(destDir, 'link')), { code: 'ENOENT' });
181+
assert.strictEqual(fs.readFileSync(path.join(outsideDir, 'target.txt'), 'utf8'), 'original');
182+
});
183+
184+
it('should preserve an internal absolute target through directory links', async () => {
185+
const destDir = path.join(tempDir, 'dest');
186+
const target = path.join(destDir, 'target.txt');
187+
await extract([
188+
{ name: 'sub/', type: 'directory' },
189+
{ name: 'sub/d', type: 'symlink', linkname: '..' },
190+
{ name: 'target.txt', content: 'safe' },
191+
{ name: 'sub/d/sub/d/link', type: 'symlink', linkname: target },
192+
], destDir);
193+
194+
const link = path.join(destDir, 'link');
195+
assert.strictEqual(fs.realpathSync(link), fs.realpathSync(target));
196+
assert.strictEqual(fs.readFileSync(link, 'utf8'), 'safe');
197+
});
198+
199+
[ 'existing', 'dangling' ].forEach(scenario => {
200+
it(`should preserve ${scenario} internal targets after moving an aliased extraction root`, async () => {
201+
const targetExists = scenario === 'existing';
202+
const realBase = path.join(tempDir, 'real');
203+
const alias = path.join(tempDir, 'alias');
204+
fs.mkdirSync(realBase);
205+
fs.symlinkSync(realBase, alias, 'dir');
206+
const destDir = path.join(alias, 'dest');
207+
const realDestDir = path.join(fs.realpathSync(realBase), 'dest');
208+
209+
const entries = [
210+
{ name: 'nested/relative', type: 'symlink', linkname: '../target.txt' },
211+
{ name: 'nested/absolute', type: 'symlink', linkname: path.join(destDir, 'target.txt') },
212+
{ name: 'nested/canonical', type: 'symlink', linkname: path.join(realDestDir, 'target.txt') },
213+
];
214+
if (targetExists) entries.unshift({ name: 'target.txt', content: 'safe' });
215+
await extract(entries, destDir);
216+
217+
const movedDir = path.join(tempDir, 'moved');
218+
fs.renameSync(destDir, movedDir);
219+
if (!targetExists) {
220+
assert.throws(() => fs.realpathSync(path.join(movedDir, 'nested/absolute')), { code: 'ENOENT' });
221+
fs.writeFileSync(path.join(movedDir, 'target.txt'), 'safe');
222+
}
223+
224+
for (const name of [ 'relative', 'absolute', 'canonical' ]) {
225+
const link = path.join(movedDir, 'nested', name);
226+
assert.strictEqual(fs.realpathSync(link), fs.realpathSync(path.join(movedDir, 'target.txt')));
227+
assert.strictEqual(fs.readFileSync(link, 'utf8'), 'safe');
228+
assert.strictEqual(fs.readlinkSync(link), path.join('..', 'target.txt'));
229+
}
230+
});
231+
});
232+
});
233+
});
234+
});

0 commit comments

Comments
0 (0)
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.