Skip to content

Commit 18def23

Browse files
fengmk2fengmk2
committed
fix: prevent symlink path traversal via pre-existing symlinks during tar extraction
Add isRealPathSafe() that walks each path segment using lstat to detect when a pre-existing symlink on disk would cause a file write outside the extraction directory. This mitigates GHSA-4c3q-x735-j3r5 where a crafted tar with ordered entries (symlink then file through that symlink) could escape the destination. Also extracts createTarBuffer helper to shared test/util.js. Closes GHSA-4c3q-x735-j3r5
1 parent 1c1b725 commit 18def23

File tree

4 files changed

+384
-91
lines changed

4 files changed

+384
-91
lines changed

‎lib/utils.js‎

Lines changed: 144 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,63 @@ function isPathWithinParent(childPath, parentPath) {
2121
normalizedChild.startsWith(parentWithSep);
2222
}
2323

24+
/**
25+
* Check if the real filesystem path stays within parentDir,
26+
* accounting for pre-existing symlinks on disk.
27+
* Walks each path segment from parentDir to targetPath using lstat.
28+
* If any segment is a symlink, resolves it and verifies it stays within parentDir.
29+
* @param {string} targetPath - Absolute path to validate
30+
* @param {string} parentDir - Absolute path of the extraction root
31+
* @param {string} realParentDir - Pre-resolved real path of parentDir (handles OS-level symlinks like /var -> /private/var on macOS)
32+
* @param {function} callback - callback(err, safe)
33+
*/
34+
function isRealPathSafe(targetPath, parentDir, realParentDir, callback) {
35+
function isWithinParent(p) {
36+
return isPathWithinParent(p, parentDir) || isPathWithinParent(p, realParentDir);
37+
}
38+
39+
var relative = path.relative(parentDir, targetPath);
40+
var segments = relative.split(path.sep);
41+
var i = 0;
42+
var current = parentDir;
43+
44+
function checkNext() {
45+
if (i >= segments.length) return callback(null, true);
46+
var segment = segments[i++];
47+
if (!segment || segment === '.') return checkNext();
48+
49+
current = path.join(current, segment);
50+
fs.lstat(current, function(err, stat) {
51+
if (err) {
52+
if (err.code === 'ENOENT') return callback(null, true); // doesn't exist yet, safe
53+
// Fail closed: unexpected filesystem errors are unsafe
54+
return callback(null, false);
55+
}
56+
if (!stat.isSymbolicLink()) return checkNext();
57+
58+
fs.realpath(current, function(err, resolved) {
59+
if (err) {
60+
if (err.code === 'ENOENT') {
61+
// Dangling symlink - check textual target
62+
return fs.readlink(current, function(err, linkTarget) {
63+
if (err) return callback(null, false);
64+
var absTarget = path.resolve(path.dirname(current), linkTarget);
65+
callback(null, isWithinParent(absTarget));
66+
});
67+
}
68+
// Fail closed: unexpected errors during symlink resolution are unsafe
69+
return callback(null, false);
70+
}
71+
if (!isWithinParent(resolved)) return callback(null, false);
72+
current = resolved;
73+
checkNext();
74+
});
75+
});
76+
}
77+
78+
checkNext();
79+
}
80+
2481
// file/fileBuffer/stream
2582
exports.sourceType = source => {
2683
if (!source) return undefined;
@@ -112,74 +169,99 @@ exports.makeUncompressFn = StreamClass => {
112169
// Resolve destDir to absolute path for security validation
113170
const resolvedDestDir = path.resolve(destDir);
114171

115-
let entryCount = 0;
116-
let successCount = 0;
117-
let isFinish = false;
118-
function done() {
119-
// resolve when both stream finish and file write finish
120-
if (isFinish && entryCount === successCount) resolve();
121-
}
122-
123-
new StreamClass(opts)
124-
.on('finish', () => {
125-
isFinish = true;
126-
done();
127-
})
128-
.on('error', reject)
129-
.on('entry', (header, stream, next) => {
130-
stream.on('end', next);
131-
const destFilePath = path.join(resolvedDestDir, header.name);
132-
const resolvedDestPath = path.resolve(destFilePath);
133-
134-
// Security: Validate that the entry path doesn't escape the destination directory
135-
if (!isPathWithinParent(resolvedDestPath, resolvedDestDir)) {
136-
console.warn(`[compressing] Skipping entry with path traversal: "${header.name}" -> "${resolvedDestPath}"`);
137-
stream.resume();
138-
return;
139-
}
140-
141-
if (header.type === 'file') {
142-
const dir = path.dirname(destFilePath);
143-
mkdirp(dir, err => {
144-
if (err) return reject(err);
145-
146-
entryCount++;
147-
pump(stream, fs.createWriteStream(destFilePath, { mode: opts.mode || header.mode }), err => {
148-
if (err) return reject(err);
149-
successCount++;
150-
done();
151-
});
152-
});
153-
} else if (header.type === 'symlink') {
154-
const dir = path.dirname(destFilePath);
155-
const target = path.resolve(dir, header.linkname);
156-
157-
// Security: Validate that the symlink target doesn't escape the destination directory
158-
if (!isPathWithinParent(target, resolvedDestDir)) {
159-
console.warn(`[compressing] Skipping symlink "${header.name}": target "${target}" escapes extraction directory`);
172+
// Resolve once for the entire extraction to handle OS-level symlinks
173+
// (e.g. /var -> /private/var on macOS)
174+
let realDestDir = resolvedDestDir;
175+
fs.realpath(resolvedDestDir, (err, resolved) => {
176+
if (!err) realDestDir = resolved;
177+
178+
let entryCount = 0;
179+
let successCount = 0;
180+
let isFinish = false;
181+
function done() {
182+
// resolve when both stream finish and file write finish
183+
if (isFinish && entryCount === successCount) resolve();
184+
}
185+
186+
new StreamClass(opts)
187+
.on('finish', () => {
188+
isFinish = true;
189+
done();
190+
})
191+
.on('error', reject)
192+
.on('entry', (header, stream, next) => {
193+
stream.on('end', next);
194+
const destFilePath = path.join(resolvedDestDir, header.name);
195+
const resolvedDestPath = path.resolve(destFilePath);
196+
197+
// Security: Validate that the entry path doesn't escape the destination directory
198+
if (!isPathWithinParent(resolvedDestPath, resolvedDestDir)) {
199+
console.warn('[compressing] Skipping entry with path traversal: "' + header.name + '" -> "' + resolvedDestPath + '"');
160200
stream.resume();
161201
return;
162202
}
163203

164-
entryCount++;
165-
166-
mkdirp(dir, err => {
167-
if (err) return reject(err);
168-
169-
const relativeTarget = path.relative(dir, target);
170-
fs.symlink(relativeTarget, destFilePath, err => {
171-
if (err) return reject(err);
172-
successCount++;
204+
// Security: Validate no pre-existing symlink in the path escapes the extraction directory
205+
isRealPathSafe(resolvedDestPath, resolvedDestDir, realDestDir, (err, safe) => {
206+
if (err || !safe) {
207+
console.warn('[compressing] Skipping entry "' + header.name + '": a symlink in its path resolves outside the extraction directory');
173208
stream.resume();
174-
});
175-
});
176-
} else { // directory
177-
mkdirp(destFilePath, err => {
178-
if (err) return reject(err);
179-
stream.resume();
209+
return;
210+
}
211+
212+
if (header.type === 'file') {
213+
const dir = path.dirname(destFilePath);
214+
mkdirp(dir, err => {
215+
if (err) return reject(err);
216+
217+
entryCount++;
218+
pump(stream, fs.createWriteStream(destFilePath, { mode: opts.mode || header.mode }), err => {
219+
if (err) return reject(err);
220+
successCount++;
221+
done();
222+
});
223+
});
224+
} else if (header.type === 'symlink') {
225+
const dir = path.dirname(destFilePath);
226+
const target = path.resolve(dir, header.linkname);
227+
228+
// Security: Validate that the symlink target doesn't escape the destination directory
229+
if (!isPathWithinParent(target, resolvedDestDir)) {
230+
console.warn('[compressing] Skipping symlink "' + header.name + '": target "' + target + '" escapes extraction directory');
231+
stream.resume();
232+
return;
233+
}
234+
235+
// Security: Validate no pre-existing symlink in the target path escapes the extraction directory
236+
isRealPathSafe(target, resolvedDestDir, realDestDir, (err, targetSafe) => {
237+
if (err || !targetSafe) {
238+
console.warn('[compressing] Skipping symlink "' + header.name + '": target resolves outside extraction directory via existing symlink');
239+
stream.resume();
240+
return;
241+
}
242+
243+
entryCount++;
244+
245+
mkdirp(dir, err => {
246+
if (err) return reject(err);
247+
248+
const relativeTarget = path.relative(dir, target);
249+
fs.symlink(relativeTarget, destFilePath, err => {
250+
if (err) return reject(err);
251+
successCount++;
252+
stream.resume();
253+
});
254+
});
255+
});
256+
} else { // directory
257+
mkdirp(destFilePath, err => {
258+
if (err) return reject(err);
259+
stream.resume();
260+
});
261+
}
180262
});
181-
}
182-
});
263+
});
264+
});
183265
});
184266
});
185267
};

0 commit comments

Comments
 (0)