Skip to content

Commit

Permalink
fix: sanitize filenames with 'loadAsync'
Browse files Browse the repository at this point in the history
  • Loading branch information
tsvetomir committed Dec 20, 2024
1 parent 21f9660 commit 99a15cc
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 1 deletion.
5 changes: 5 additions & 0 deletions index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,11 @@ export type OutputType = keyof OutputByType;

export interface JSZipObject {
name: string;
/**
* Present for files loadded with `loadAsync`. May contain ".." path components that could
* result in a zip-slip attack. See https://snyk.io/research/zip-slip-vulnerability
*/
unsafeOriginalName?: string;
dir: boolean;
date: Date;
comment: string;
Expand Down
9 changes: 8 additions & 1 deletion lib/load.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,11 @@ export default function(data, options) {
var files = zipEntries.files;
for (var i = 0; i < files.length; i++) {
var input = files[i];
zip.file(input.fileNameStr, input.decompressed, {

var unsafeName = input.fileNameStr;
var safeName = utils.resolve(input.fileNameStr);

zip.file(safeName, input.decompressed, {
binary: true,
optimizedBinaryString: true,
date: input.date,
Expand All @@ -72,6 +76,9 @@ export default function(data, options) {
dosPermissions : input.dosPermissions,
createFolders: options.createFolders
});
if (!input.dir) {
zip.file(safeName).unsafeOriginalName = unsafeName;
}
}
if (zipEntries.zipComment.length) {
zip.comment = zipEntries.zipComment;
Expand Down
25 changes: 25 additions & 0 deletions lib/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,31 @@ export const transformTo = function(outputType, input) {
return result;
};

/**
* Resolve all relative path components, "." and "..", in a path. If these relative components
* traverse above the root then the resulting path will only contain the final path component.
*
* All empty components, e.g. "//", are removed.
* @param {string} path A path with / or \ separators
* @returns {string} The path with all relative path components resolved.
*/
export const resolve = function(path) {
var parts = path.split("/");
var result = [];
for (var index = 0; index < parts.length; index++) {
var part = parts[index];
// Allow the first and last component to be empty for trailing slashes.
if (part === "." || (part === "" && index !== 0 && index !== parts.length - 1)) {
continue;
} else if (part === "..") {
result.pop();
} else {
result.push(part);
}
}
return result.join("/");
};

/**
* Return the type of the input.
* The type will be in a format valid for JSZip.utils.transformTo : string, array, uint8array, arraybuffer.
Expand Down
36 changes: 36 additions & 0 deletions test/asserts/utils.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/* global QUnit,JSZip,JSZipTestUtils,Promise */
'use strict';

QUnit.module("utils");

function resolve(path) {
var parts = path.split("/");
var result = [];
for (var index = 0; index < parts.length; index++) {
var part = parts[index];
// Allow the first and last component to be empty for trailing slashes.
if (part === "." || (part === "" && index !== 0 && index !== parts.length - 1)) {
continue;
} else if (part === "..") {
result.pop();
} else {
result.push(part);
}
}
return result.join("/");
};

QUnit.test("Paths are resolved correctly", function (assert) {
// Backslashes can be part of filenames
assert.strictEqual(resolve("root\\a\\b"), "root\\a\\b");
assert.strictEqual(resolve("root/a/b"), "root/a/b");
assert.strictEqual(resolve("root/a/.."), "root");
assert.strictEqual(resolve("root/a/../b"), "root/b");
assert.strictEqual(resolve("root/a/./b"), "root/a/b");
assert.strictEqual(resolve("root/../../../"), "");
assert.strictEqual(resolve("////"), "/");
assert.strictEqual(resolve("/a/b/c"), "/a/b/c");
assert.strictEqual(resolve("a/b/c/"), "a/b/c/");
assert.strictEqual(resolve("../../../../../a"), "a");
assert.strictEqual(resolve("../app.js"), "app.js");
});
1 change: 1 addition & 0 deletions test/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
<script type="text/javascript" src="asserts/permissions.js"></script>
<script type="text/javascript" src="asserts/stream.js"></script>
<script type="text/javascript" src="asserts/unicode.js"></script>
<script type="text/javascript" src="asserts/utils.js"></script>

</head>
<body>
Expand Down

0 comments on commit 99a15cc

Please sign in to comment.