Merge pull request #381 from 5saviahv/sanitize

5saviahv · web-flow · commit 775165d87138 · 2021-05-24T19:15:17.000+03:00
Move sanitize functions to Utils
diff --git a/adm-zip.js b/adm-zip.js
@@ -15,12 +15,6 @@ const defaultOptions = {
     method: Utils.Constants.NONE
 };
 
-function canonical(p) {
-    // trick normalize think path is absolute
-    var safeSuffix = pth.posix.normalize("/" + p.split("\\").join("/"));
-    return pth.join(".", safeSuffix);
-}
-
 module.exports = function (/**String*/ input, /** object */ options) {
     let inBuffer = null;
 
@@ -37,7 +31,7 @@ module.exports = function (/**String*/ input, /** object */ options) {
         }
 
         // if input is buffer
-        if (input instanceof Uint8Array) {
+        if (Buffer.isBuffer(input)) {
             inBuffer = input;
             opts.method = Utils.Constants.BUFFER;
             input = undefined;
@@ -62,17 +56,7 @@ module.exports = function (/**String*/ input, /** object */ options) {
     // create variable
     const _zip = new ZipFile(inBuffer, opts);
 
-    function sanitize(prefix, name) {
-        prefix = pth.resolve(pth.normalize(prefix));
-        var parts = name.split("/");
-        for (var i = 0, l = parts.length; i < l; i++) {
-            var path = pth.normalize(pth.join(prefix, parts.slice(i, l).join(pth.sep)));
-            if (path.indexOf(prefix) === 0) {
-                return path;
-            }
-        }
-        return pth.normalize(pth.join(prefix, pth.basename(name)));
-    }
+    const { canonical, sanitize } = Utils;
 
     function getEntry(/**Object*/ entry) {
         if (entry && _zip) {
@@ -636,48 +620,52 @@ module.exports = function (/**String*/ input, /** object */ options) {
                 return;
             }
 
-            var entries = _zip.entries;
-            var i = entries.length;
-            entries.forEach(function (entry) {
-                if (i <= 0) return; // Had an error already
+            targetPath = pth.resolve(targetPath);
+            // convert entryname to
+            const getPath = (entry) => sanitize(targetPath, pth.normalize(canonical(entry.entryName.toString())));
+            const getError = (msg, file) => new Error(msg + ': "' + file + '"');
 
-                var entryName = pth.normalize(canonical(entry.entryName.toString()));
+            const entries = _zip.entries;
 
-                if (entry.isDirectory) {
-                    Utils.makeDir(sanitize(targetPath, entryName));
-                    if (--i === 0) callback(undefined);
-                    return;
+            // Create directory entries first
+            for (const entry of entries.filter((e) => e.isDirectory)) {
+                const filePath = getPath(entry);
+                // The reverse operation for attr depend on method addFile()
+                const fileAttr = entry.header.fileAttr;
+                try {
+                    Utils.makeDir(filePath);
+                    if (fileAttr) fs.chmodSync(filePath, fileAttr);
+                    // in unix timestamp will change if files are later added to folder, but still
+                    fs.utimesSync(filePath, entry.header.time, entry.header.time);
+                } catch (er) {
+                    callback(getError("Unable to create folder", filePath));
                 }
-                entry.getDataAsync(function (content, err) {
-                    if (i <= 0) return;
-                    if (err) {
-                        callback(new Error(err));
+            }
+
+            // File entries
+            for (const entry of entries.filter((e) => !e.isDirectory)) {
+                const entryName = pth.normalize(canonical(entry.entryName.toString()));
+                const filePath = sanitize(targetPath, entryName);
+                entry.getDataAsync(function (content, err_1) {
+                    if (err_1) {
+                        callback(new Error(err_1));
                         return;
                     }
                     if (!content) {
-                        i = 0;
                         callback(new Error(Utils.Errors.CANT_EXTRACT_FILE));
-                        return;
+                    } else {
+                        // The reverse operation for attr depend on method addFile()
+                        Utils.writeFileToAsync(filePath, content, overwrite, entry.header.fileAttr, function (succ) {
+                            if (!succ) callback(getError("Unable to write file", filePath));
+                            fs.utimes(filePath, entry.header.time, entry.header.time, function (err_2) {
+                                if (err_2) callback(getError("Unable to set times", filePath));
+                            });
+                        });
                     }
-
-                    // The reverse operation for attr depend on method addFile()
-                    var fileAttr = entry.attr ? (((entry.attr >>> 0) | 0) >> 16) & 0xfff : 0;
-                    Utils.writeFileToAsync(sanitize(targetPath, entryName), content, overwrite, fileAttr, function (succ) {
-                        try {
-                            fs.utimesSync(pth.resolve(targetPath, entryName), entry.header.time, entry.header.time);
-                        } catch (er) {
-                            callback(new Error("Unable to set utimes"));
-                        }
-                        if (i <= 0) return;
-                        if (!succ) {
-                            i = 0;
-                            callback(new Error("Unable to write"));
-                            return;
-                        }
-                        if (--i === 0) callback(undefined);
-                    });
                 });
-            });
+            }
+
+            callback();
         },
 
         /**
diff --git a/headers/entryHeader.js b/headers/entryHeader.js
@@ -84,21 +84,21 @@ module.exports = function () {
             return _crc;
         },
         set crc(val) {
-            _crc = val;
+            _crc = Math.max(0, val) >>> 0;
         },
 
         get compressedSize() {
             return _compressedSize;
         },
         set compressedSize(val) {
-            _compressedSize = val;
+            _compressedSize = Math.max(0, val) >>> 0;
         },
 
         get size() {
             return _size;
         },
         set size(val) {
-            _size = val;
+            _size = Math.max(0, val) >>> 0;
         },
 
         get fileNameLength() {
@@ -126,28 +126,33 @@ module.exports = function () {
             return _diskStart;
         },
         set diskNumStart(val) {
-            _diskStart = val;
+            _diskStart = Math.max(0, val) >>> 0;
         },
 
         get inAttr() {
             return _inattr;
         },
         set inAttr(val) {
-            _inattr = val;
+            _inattr = Math.max(0, val) >>> 0;
         },
 
         get attr() {
             return _attr;
         },
         set attr(val) {
-            _attr = val;
+            _attr = Math.max(0, val) >>> 0;
+        },
+
+        // get Unix file permissions
+        get fileAttr() {
+            return (((_attr >>> 0) | 0) >> 16) & 0xfff;
         },
 
         get offset() {
             return _offset;
         },
         set offset(val) {
-            _offset = val;
+            _offset = Math.max(0, val) >>> 0;
         },
 
         get encripted() {
diff --git a/test/utils.test.js b/test/utils.test.js
@@ -1,8 +1,9 @@
 "use strict";
 const { expect } = require("chai");
-const { crc32 } = require("../util/utils");
+const { crc32, canonical, sanitize } = require("../util/utils");
+const pth = require("path");
 
-describe("crc32 function", () => {
+describe("utils - crc32 function", () => {
     // tests how crc32 function handles strings as input
     it("handle strings", () => {
         const tests = [
@@ -34,3 +35,58 @@ describe("crc32 function", () => {
         }
     });
 });
+
+describe("utils - sanitizing functions :", () => {
+    // tests how sanitize works
+    it("function sanitize()", () => {
+        const tests = [
+            // basic latin
+            { prefix: "", file: "", result: "" },
+            { prefix: "folder", file: "file", result: "folder/file" },
+            { prefix: "folder", file: "../file", result: "folder/file" },
+            { prefix: "folder", file: "../../../file", result: "folder/file" },
+            { prefix: "folder", file: "./../file", result: "folder/file" },
+            { prefix: "test/folder/subfolder", file: "../../file", result: "test/folder/subfolder/file" },
+            { prefix: "test/folder/subfolder", file: "../../file1/../file2", result: "test/folder/subfolder/file2" },
+            // no prefixed (currently allows change folder)
+            { prefix: "", file: "../../file1/../file2", result: "file2" },
+            { prefix: "", file: "../subfolder/file2", result: "subfolder/file2" },
+            { prefix: "", file: "../subfolder2/file2", result: "subfolder2/file2" },
+            { prefix: "", file: "../subfolder/file2", result: "subfolder/file2" },
+            { prefix: "", file: "../../subfolder2/file2", result: "subfolder2/file2" }
+        ];
+
+        const curfolder = pth.resolve(".");
+        // console.log("\n");
+        for (let test of tests) {
+            // path.normalize in win32 will convert "/" to native "\" format
+
+            const out = sanitize(pth.normalize(test.prefix || ""), test.file);
+            const res = pth.join(curfolder, pth.normalize(test.result));
+
+            expect(out).to.equal(res);
+        }
+    });
+
+    it("function canonical()", () => {
+        const tests = [
+            // no name
+            { file: "", result: "" },
+            // file has name
+            { file: "file", result: "file" },
+            { file: "../file", result: "file" },
+            { file: "../../../file", result: "file" },
+            { file: "./../file", result: "file" },
+            { file: "../../file", result: "file" },
+            { file: "../../file1/../file2", result: "file2" },
+            { file: "../subfolder/file2", result: "subfolder/file2" },
+            { file: "../subfolder2/file2", result: "subfolder2/file2" },
+            { file: "../subfolder/file2", result: "subfolder/file2" },
+            { file: "../../subfolder2/file2", result: "subfolder2/file2" }
+        ];
+
+        for (let test of tests) {
+            expect(canonical(test.file)).to.equal(test.result);
+        }
+    });
+});
diff --git a/util/utils.js b/util/utils.js
@@ -123,6 +123,25 @@ module.exports = (function () {
             return true;
         },
 
+        canonical: function (p) {
+            if (!p) return "";
+            // trick normalize think path is absolute
+            var safeSuffix = pth.posix.normalize("/" + p.split("\\").join("/"));
+            return pth.join(".", safeSuffix);
+        },
+
+        sanitize: function (prefix, name) {
+            prefix = pth.resolve(pth.normalize(prefix));
+            var parts = name.split("/");
+            for (var i = 0, l = parts.length; i < l; i++) {
+                var path = pth.normalize(pth.join(prefix, parts.slice(i, l).join(pth.sep)));
+                if (path.indexOf(prefix) === 0) {
+                    return path;
+                }
+            }
+            return pth.normalize(pth.join(prefix, pth.basename(name)));
+        },
+
         writeFileToAsync: function (/*String*/ path, /*Buffer*/ content, /*Boolean*/ overwrite, /*Number*/ attr, /*Function*/ callback) {
             if (typeof attr === "function") {
                 callback = attr;
diff --git a/zipFile.js b/zipFile.js
