Skip to content

Commit 07d6423

Browse files
ayuhitoayuhito
authored
fix(unpack): strip trailing slashes (#85)
* fix(unpack): strip trailing slashes * fix(unpack): catch windows normalize errors * fix(unpack): let abort controller destroy streams * style: remove unnecessary
1 parent 2be34a3 commit 07d6423

File tree

5 files changed

+466
-150
lines changed

5 files changed

+466
-150
lines changed

‎src/fs/path.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import * as path from "node:path";
2+
import { normalizeWindowsPath } from "./win-path";
23

34
const unicodeCache = new Map<string, string>();
45

@@ -21,6 +22,25 @@ export const normalizeUnicode = (s: string): string => {
2122
return result;
2223
};
2324

25+
// Strips trailing slashes from a path.
26+
export function stripTrailingSlashes(p: string): string {
27+
let i = p.length - 1;
28+
if (i === -1 || p[i] !== "/") {
29+
return p;
30+
}
31+
32+
let slashesStart = i;
33+
while (i > -1 && p[i] === "/") {
34+
slashesStart = i;
35+
i--;
36+
}
37+
38+
return p.slice(0, slashesStart);
39+
}
40+
41+
export const normalizeHeaderName = (s: string) =>
42+
normalizeUnicode(normalizeWindowsPath(stripTrailingSlashes(s)));
43+
2444
// Validates that the given target path is within the destination directory and does not escape.
2545
export function validateBounds(
2646
targetPath: string,

‎src/fs/unpack.ts‎

Lines changed: 140 additions & 145 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,8 @@ import { pipeline } from "node:stream/promises";
77
import { transformHeader } from "../tar/options";
88
import type { TarHeader } from "../tar/types";
99
import { createTarUnpacker } from "../tar/unpacker";
10-
import { normalizeUnicode, validateBounds } from "./path";
10+
import { normalizeHeaderName, normalizeUnicode, validateBounds } from "./path";
1111
import type { UnpackOptionsFS } from "./types";
12-
import { normalizeWindowsPath } from "./win-path";
1312

1413
/**
1514
* Extract a tar archive to a directory.
@@ -169,12 +168,16 @@ function createFSHandler(directoryPath: string, options: UnpackOptionsFS) {
169168

170169
// If the directory is the destination directory, it already exists.
171170
promise = (async (): Promise<TarHeader["type"]> => {
171+
if (signal.aborted) throw signal.reason;
172+
172173
const destDir = await destDirPromise;
173174
if (dirPath === destDir.symbolic) return "directory";
174175

175176
// Ensure parent directory exists first.
176177
await ensureDirectoryExists(path.dirname(dirPath));
177178

179+
if (signal.aborted) throw signal.reason;
180+
178181
// Check if the directory exists.
179182
try {
180183
await fs.mkdir(dirPath, { mode: dmode });
@@ -204,130 +207,6 @@ function createFSHandler(directoryPath: string, options: UnpackOptionsFS) {
204207
return promise;
205208
};
206209

207-
const processHeader = async (
208-
header: TarHeader,
209-
entryStream: PassThrough,
210-
): Promise<TarHeader["type"]> => {
211-
try {
212-
// Await the destination directory to ensure it's created first.
213-
const destDir = await destDirPromise;
214-
const normalizedName = normalizeUnicode(
215-
normalizeWindowsPath(header.name),
216-
);
217-
218-
// Prevent ReDOS via deep paths.
219-
if (maxDepth !== Infinity && normalizedName.split("/").length > maxDepth)
220-
throw new Error("Tar exceeds max specified depth.");
221-
222-
// Prevent absolute paths and ensure within destDir.
223-
if (path.isAbsolute(normalizedName))
224-
throw new Error(`Absolute path found in "${header.name}".`);
225-
226-
const outPath = path.join(destDir.symbolic, normalizedName);
227-
validateBounds(
228-
outPath,
229-
destDir.symbolic,
230-
`Entry "${header.name}" points outside the extraction directory.`,
231-
);
232-
233-
// Ensure parent directory exists.
234-
const parentDir = path.dirname(outPath);
235-
await ensureDirectoryExists(parentDir);
236-
237-
switch (header.type) {
238-
case "directory":
239-
await fs.mkdir(outPath, {
240-
recursive: true,
241-
mode: dmode ?? header.mode,
242-
});
243-
break;
244-
245-
case "file": {
246-
const fileStream = createWriteStream(outPath, {
247-
mode: fmode ?? header.mode,
248-
// Use 512KB buffer for files > 1MB.
249-
highWaterMark: header.size > 1048576 ? 524288 : undefined,
250-
});
251-
await pipeline(entryStream, fileStream);
252-
break;
253-
}
254-
255-
case "symlink": {
256-
const { linkname } = header;
257-
if (!linkname) return header.type;
258-
const target = path.resolve(parentDir, linkname);
259-
validateBounds(
260-
target,
261-
destDir.symbolic,
262-
`Symlink "${linkname}" points outside the extraction directory.`,
263-
);
264-
await fs.symlink(linkname, outPath);
265-
break;
266-
}
267-
268-
case "link": {
269-
const { linkname } = header;
270-
if (!linkname) return header.type;
271-
272-
// Resolve the hardlink target path and ensure it's within destDir.
273-
const normalizedLink = normalizeUnicode(linkname);
274-
if (path.isAbsolute(normalizedLink)) {
275-
throw new Error(
276-
`Hardlink "${linkname}" points outside the extraction directory.`,
277-
);
278-
}
279-
280-
// This is the symbolic path to the link's target inside the extraction dir.
281-
const linkTarget = path.join(destDir.symbolic, normalizedLink);
282-
validateBounds(
283-
linkTarget,
284-
destDir.symbolic,
285-
`Hardlink "${linkname}" points outside the extraction directory.`,
286-
);
287-
await ensureDirectoryExists(path.dirname(linkTarget));
288-
289-
// Resolve the real path of the parent directory which follows symlinks.
290-
const realTargetParent = await fs.realpath(path.dirname(linkTarget));
291-
const realLinkTarget = path.join(
292-
realTargetParent,
293-
path.basename(linkTarget),
294-
);
295-
296-
// Check that the real path is within the destination directory.
297-
validateBounds(
298-
realLinkTarget,
299-
destDir.real,
300-
`Hardlink "${linkname}" points outside the extraction directory.`,
301-
);
302-
303-
// A self-referential hardlink should be a noop.
304-
if (linkTarget === outPath) return header.type;
305-
306-
// Wait for the target to be created if it is in the map.
307-
const targetPromise = pathPromises.get(linkTarget);
308-
if (targetPromise) await targetPromise;
309-
310-
await fs.link(linkTarget, outPath);
311-
break;
312-
}
313-
314-
default:
315-
return header.type; // Unsupported type
316-
}
317-
318-
// Set modification time if available.
319-
if (header.mtime) {
320-
const utimes = header.type === "symlink" ? fs.lutimes : fs.utimes;
321-
await utimes(outPath, header.mtime, header.mtime).catch(() => {});
322-
}
323-
324-
return header.type;
325-
} finally {
326-
// Ensure the entry stream is drained to avoid blocking.
327-
if (!entryStream.readableEnded) entryStream.resume();
328-
}
329-
};
330-
331210
const handler = {
332211
onHeader(header: TarHeader) {
333212
if (signal.aborted) return;
@@ -350,21 +229,13 @@ function createFSHandler(directoryPath: string, options: UnpackOptionsFS) {
350229
return;
351230
}
352231

353-
const destDir = path.resolve(directoryPath);
354-
355-
// Ensure that "path" and "path/" are treated as the same key on all platforms.
356-
const keyPath = path.join(
357-
destDir,
358-
normalizeUnicode(transformed.name),
359-
);
360-
const normalizedTarget =
361-
keyPath.endsWith("/") || keyPath.endsWith("\\")
362-
? keyPath.slice(0, -1)
363-
: keyPath;
232+
// Normalize and resolve the target path.
233+
const name = normalizeHeaderName(transformed.name);
234+
const target = path.join(path.resolve(directoryPath), name);
364235

365236
// Chain onto any prior operation for this path.
366237
const priorOpPromise =
367-
pathPromises.get(normalizedTarget) || Promise.resolve(undefined);
238+
pathPromises.get(target) || Promise.resolve(undefined);
368239

369240
// Start the operation promise chain.
370241
opPromise = priorOpPromise.then(async (priorOp) => {
@@ -374,20 +245,145 @@ function createFSHandler(directoryPath: string, options: UnpackOptionsFS) {
374245
(priorOp === "directory" && transformed.type !== "directory") ||
375246
(priorOp !== "directory" && transformed.type === "directory");
376247

377-
if (isConflict) {
248+
if (isConflict)
378249
throw new Error(
379-
`Path conflict: cannot create ${transformed.type} over existing ${priorOp} at "${transformed.name}"`,
250+
`Path conflict ${transformed.type} over existing ${priorOp} at "${transformed.name}"`,
380251
);
381-
}
382252
}
383253

384-
return await processHeader(transformed, entryStream);
254+
try {
255+
const destDir = await destDirPromise;
256+
257+
// Prevent ReDOS via deep paths.
258+
if (maxDepth !== Infinity && name.split("/").length > maxDepth)
259+
throw new Error("Tar exceeds max specified depth.");
260+
261+
// Prevent absolute paths and ensure within destDir.
262+
if (path.isAbsolute(name))
263+
throw new Error(
264+
`Absolute path found in "${transformed.name}".`,
265+
);
266+
267+
const outPath = path.join(destDir.symbolic, name);
268+
validateBounds(
269+
outPath,
270+
destDir.symbolic,
271+
`Entry "${transformed.name}" points outside the extraction directory.`,
272+
);
273+
274+
// Ensure parent directory exists.
275+
const parentDir = path.dirname(outPath);
276+
await ensureDirectoryExists(parentDir);
277+
278+
switch (transformed.type) {
279+
case "directory":
280+
await fs.mkdir(outPath, {
281+
recursive: true,
282+
mode: dmode ?? transformed.mode,
283+
});
284+
break;
285+
286+
case "file": {
287+
const fileStream = createWriteStream(outPath, {
288+
mode: fmode ?? transformed.mode,
289+
// Use 512KB buffer for files > 1MB.
290+
highWaterMark:
291+
transformed.size > 1048576 ? 524288 : undefined,
292+
});
293+
await pipeline(entryStream, fileStream);
294+
break;
295+
}
296+
297+
case "symlink": {
298+
const { linkname } = transformed;
299+
if (!linkname) return transformed.type;
300+
const target = path.resolve(parentDir, linkname);
301+
validateBounds(
302+
target,
303+
destDir.symbolic,
304+
`Symlink "${linkname}" points outside the extraction directory.`,
305+
);
306+
await fs.symlink(linkname, outPath);
307+
break;
308+
}
309+
310+
case "link": {
311+
const { linkname } = transformed;
312+
if (!linkname) return transformed.type;
313+
314+
// Resolve the hardlink target path and ensure it's within destDir.
315+
const normalizedLink = normalizeUnicode(linkname);
316+
if (path.isAbsolute(normalizedLink)) {
317+
throw new Error(
318+
`Hardlink "${linkname}" points outside the extraction directory.`,
319+
);
320+
}
321+
322+
// This is the symbolic path to the link's target inside the extraction dir.
323+
const linkTarget = path.join(
324+
destDir.symbolic,
325+
normalizedLink,
326+
);
327+
validateBounds(
328+
linkTarget,
329+
destDir.symbolic,
330+
`Hardlink "${linkname}" points outside the extraction directory.`,
331+
);
332+
await ensureDirectoryExists(path.dirname(linkTarget));
333+
334+
// Resolve the real path of the parent directory which follows symlinks.
335+
const realTargetParent = await fs.realpath(
336+
path.dirname(linkTarget),
337+
);
338+
const realLinkTarget = path.join(
339+
realTargetParent,
340+
path.basename(linkTarget),
341+
);
342+
343+
// Check that the real path is within the destination directory.
344+
validateBounds(
345+
realLinkTarget,
346+
destDir.real,
347+
`Hardlink "${linkname}" points outside the extraction directory.`,
348+
);
349+
350+
// A self-referential hardlink should be a noop.
351+
if (linkTarget === outPath) return transformed.type;
352+
353+
// Wait for the target to be created if it is in the map.
354+
const targetPromise = pathPromises.get(linkTarget);
355+
if (targetPromise) await targetPromise;
356+
357+
await fs.link(linkTarget, outPath);
358+
break;
359+
}
360+
361+
default:
362+
return transformed.type; // Unsupported type
363+
}
364+
365+
// Set modification time if available.
366+
if (transformed.mtime) {
367+
const utimes =
368+
transformed.type === "symlink" ? fs.lutimes : fs.utimes;
369+
await utimes(
370+
outPath,
371+
transformed.mtime,
372+
transformed.mtime,
373+
).catch(() => {});
374+
}
375+
376+
return transformed.type;
377+
} finally {
378+
// Ensure the entry stream is drained to avoid blocking.
379+
if (!entryStream.readableEnded) entryStream.resume();
380+
}
385381
});
386-
pathPromises.set(normalizedTarget, opPromise);
382+
383+
pathPromises.set(target, opPromise);
387384
} catch (err) {
388385
opPromise = Promise.reject(err);
389386
abortController.abort(err as Error);
390-
entryStream?.destroy(err as Error);
391387
}
392388

393389
opPromise
@@ -413,7 +409,6 @@ function createFSHandler(directoryPath: string, options: UnpackOptionsFS) {
413409

414410
onError(error: Error) {
415411
abortController.abort(error);
416-
activeEntryStream?.destroy(error);
417412
},
418413

419414
async process() {

0 commit comments

Comments
 (0)