Merge pull request #813 from Stuk/santize-loaded-filenames

v3.8.0 Santize loaded filenames
Stuk · Mar 30, 2022 · 121eec0 · 121eec0
2 parents d4702a7 + 3b98cfc
commit 121eec0
Show file tree

Hide file tree

Showing 12 changed files with 135 additions and 16 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -4,10 +4,14 @@ layout: default
 section: main
 ---
 
+### v3.8.0 2022-03-30
+
+- Santize filenames when files are loaded with `loadAsync`, to avoid ["zip slip" attacks](https://snyk.io/research/zip-slip-vulnerability). The original filename is available on each zip entry as `unsafeOriginalName`. See the [documentation](https://stuk.github.io/jszip/documentation/api_jszip/load_async.html). Many thanks to McCaulay Hudson for reporting.
+
 ### v3.7.1 2021-08-05
 
 - Fix build of `dist` files.
-    + Note: this version ensures the changes from 3.7.0 are actually included in the `dist` files. Thanks to Evan W for reporting. 
+    + Note: this version ensures the changes from 3.7.0 are actually included in the `dist` files. Thanks to Evan W for reporting.
 
 ### v3.7.0 2021-07-23
 

diff --git a/dist/jszip.js b/dist/jszip.js
@@ -1,6 +1,6 @@
 /*!
 
-JSZip v3.7.1 - A JavaScript class for generating and reading zip files
+JSZip v3.8.0 - A JavaScript class for generating and reading zip files
 <http://stuartk.com/jszip>
 
 (c) 2009-2016 Stuart Knightley <stuart [at] stuartk.com>
@@ -1059,7 +1059,7 @@ JSZip.defaults = require('./defaults');
 
 // TODO find a better way to handle this version,
 // a require('package.json').version doesn't work with webpack, see #327
-JSZip.version = "3.7.1";
+JSZip.version = "3.8.0";
 
 JSZip.loadAsync = function (content, options) {
     return new JSZip().loadAsync(content, options);
@@ -1132,7 +1132,11 @@ module.exports = 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,
@@ -1142,6 +1146,9 @@ module.exports = 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;
@@ -3352,6 +3359,31 @@ exports.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.
+ */
+exports.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.
@@ -3460,8 +3492,8 @@ exports.prepareContent = function(name, inputData, isBinary, isOptimizedBinarySt
 
     // if inputData is already a promise, this flatten it.
     var promise = external.Promise.resolve(inputData).then(function(data) {
-        
-        
+
+
         var isBlob = support.blob && (data instanceof Blob || ['[object File]', '[object Blob]'].indexOf(Object.prototype.toString.call(data)) !== -1);
 
         if (isBlob && typeof FileReader !== "undefined") {

diff --git a/dist/jszip.min.js b/dist/jszip.min.js
diff --git a/documentation/api_jszip/load_async.md b/documentation/api_jszip/load_async.md
@@ -10,6 +10,8 @@ object at the current folder level. This technique has some limitations, see
 If the JSZip object already contains entries, new entries will be merged. If
 two have the same name, the loaded one will replace the other.
 
+Since v3.8.0 this method will santize relative path components (i.e. `..`) in loaded filenames to avoid ["zip slip" attacks](https://snyk.io/research/zip-slip-vulnerability). For example: `../../../example.txt` → `example.txt`, `src/images/../example.txt` → `src/example.txt`. The original filename is available on each zip entry as `unsafeOriginalName`.
+
 __Returns__ : A [Promise](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise) with the updated zip object.
 The promise can fail if the loaded data is not valid zip data or if it
 uses unsupported features (multi volume, password protected, etc).
@@ -194,3 +196,24 @@ zip.loadAsync(bin1)
     // file3.txt, from bin2
 });
 ```
+
+Reading a zip file with relative filenames:
+
+```js
+// here, "unsafe.zip" is zip file containing:
+// src/images/../file.txt
+// ../../example.txt
+
+require("fs").readFile("unsafe.zip", function (err, data) {
+    if (err) throw err;
+    var zip = new JSZip();
+    zip.loadAsync(data)
+    .then(function (zip) {
+        console.log(zip.files);
+        // src/file.txt
+        // example.txt
+        console.log(zip.files["example.txt"].unsafeOriginalName);
+        // "../../example.txt"
+    });
+}
+```
diff --git a/documentation/contributing.md b/documentation/contributing.md
@@ -63,14 +63,15 @@ If you have tested bug fixes or new features, you can open a
 
 ## Releasing a new version
 
-1. In `package.json` temporarily change `"./lib/index"` to `"."`
+1. In `package.json` temporarily remove `browser["./lib/index"]`.
 2. Run `npm test`
     * Locally open http://localhost:8080/test/
     * Or use the SauceLabs configuration above
 3. Update `JSZip.version` in `index.js` and in `package.json`
 4. Run `grunt` to generate the new dist files
     * undo the package.json change, it was just needed to replace the `__VERSION__` in the header
-5. Undo step 1.
+5. Undo step 1. and change version back.
 6. Update CHANGES.md
+6. Commit the appropriate changes
 7. Run `npm version ...` where `...` is `major`, `minor`, or `patch`
 8. Run npm publish
diff --git a/index.d.ts b/index.d.ts
@@ -65,6 +65,11 @@ declare namespace JSZip {
 
     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;

diff --git a/lib/index.js b/lib/index.js
@@ -45,7 +45,7 @@ JSZip.defaults = require('./defaults');
 
 // TODO find a better way to handle this version,
 // a require('package.json').version doesn't work with webpack, see #327
-JSZip.version = "3.7.1";
+JSZip.version = "3.8.0";
 
 JSZip.loadAsync = function (content, options) {
     return new JSZip().loadAsync(content, options);

diff --git a/lib/load.js b/lib/load.js
@@ -61,7 +61,11 @@ module.exports = 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,
@@ -71,6 +75,9 @@ module.exports = 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;

diff --git a/lib/utils.js b/lib/utils.js
@@ -317,6 +317,31 @@ exports.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.
+ */
+exports.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.
@@ -425,8 +450,8 @@ exports.prepareContent = function(name, inputData, isBinary, isOptimizedBinarySt
 
     // if inputData is already a promise, this flatten it.
     var promise = external.Promise.resolve(inputData).then(function(data) {
-        
-        
+
+
         var isBlob = support.blob && (data instanceof Blob || ['[object File]', '[object Blob]'].indexOf(Object.prototype.toString.call(data)) !== -1);
 
         if (isBlob && typeof FileReader !== "undefined") {

diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "jszip",
-  "version": "3.7.1",
+  "version": "3.8.0",
   "author": "Stuart Knightley <stuart@stuartk.com>",
   "description": "Create, read and edit .zip files with JavaScript http://stuartk.com/jszip",
   "scripts": {

diff --git a/test/asserts/utils.js b/test/asserts/utils.js
@@ -0,0 +1,22 @@
+/* global QUnit,JSZip,JSZipTestUtils */
+'use strict';
+
+// These tests only run in Node
+var utils = require("../../lib/utils");
+
+QUnit.module("utils");
+
+QUnit.test("Paths are resolved correctly", function (assert) {
+    // Backslashes can be part of filenames
+    assert.strictEqual(utils.resolve("root\\a\\b"), "root\\a\\b");
+    assert.strictEqual(utils.resolve("root/a/b"), "root/a/b");
+    assert.strictEqual(utils.resolve("root/a/.."), "root");
+    assert.strictEqual(utils.resolve("root/a/../b"), "root/b");
+    assert.strictEqual(utils.resolve("root/a/./b"), "root/a/b");
+    assert.strictEqual(utils.resolve("root/../../../"), "");
+    assert.strictEqual(utils.resolve("////"), "/");
+    assert.strictEqual(utils.resolve("/a/b/c"), "/a/b/c");
+    assert.strictEqual(utils.resolve("a/b/c/"), "a/b/c/");
+    assert.strictEqual(utils.resolve("../../../../../a"), "a");
+    assert.strictEqual(utils.resolve("../app.js"), "app.js");
+});