From 00ef4fefdde5a83b79d1d8d2e6a3f07742d4981f Mon Sep 17 00:00:00 2001 From: leopf Date: Sat, 3 Jul 2021 19:50:34 +0200 Subject: [PATCH 1/4] Created test for missing directory --- test.ts | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/test.ts b/test.ts index a966c88..2d8c835 100644 --- a/test.ts +++ b/test.ts @@ -4,12 +4,18 @@ import { assertEquals } from "https://deno.land/std@0.74.0/testing/asserts.ts"; import { JSZip, readZip, zipDir } from "./mod.ts"; // FIXME use tmp directory and clean up. -async function exampleZip(path: string) { +async function exampleZip(path: string, createDirectories = true) { const zip = new JSZip(); zip.addFile("Hello.txt", "Hello World\n"); - const img = zip.folder("images"); - img.addFile("smile.gif", "\0", { base64: true }); + if (createDirectories) { + const img = zip.folder("images"); + img.addFile("smile.gif", "\0", { base64: true }); + } + else { + // Use backslashed for edge case where directory is missing. + zip.addFile("images\\smile.gif", "\0", { base64: true }) + } await zip.writeZip(path); } @@ -75,3 +81,17 @@ Deno.test("unzip", async () => { const smile = await Deno.readFile(join(dir, "images", "smile.gif")); assertEquals("", decode(smile)); }); + +Deno.test("unzip without dir", async () => { + const dir = await Deno.makeTempDir(); + + await exampleZip("example.zip", false); + const z = await readZip("example.zip"); + await z.unzip(dir); + + const content = await Deno.readFile(join(dir, "Hello.txt")); + assertEquals("Hello World\n", decode(content)); + + const smile = await Deno.readFile(join(dir, "images", "smile.gif")); + assertEquals("", decode(smile)); +}); \ No newline at end of file From 3a21183aa957b47ae8121e5ca8f79d5d3caa3d67 Mon Sep 17 00:00:00 2001 From: leopf Date: Sat, 3 Jul 2021 20:00:52 +0200 Subject: [PATCH 2/4] fixed issue with missing directory in unzip --- mod.ts | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/mod.ts b/mod.ts index d5a5a5d..5109ee8 100644 --- a/mod.ts +++ b/mod.ts @@ -1,6 +1,6 @@ import _JSZip from "https://dev.jspm.io/jszip@3.5.0"; import { WalkOptions, walk } from "https://deno.land/std@0.99.0/fs/walk.ts"; -import { SEP, join } from "https://deno.land/std@0.99.0/path/mod.ts"; +import { SEP, join, resolve, dirname } from "https://deno.land/std@0.99.0/path/mod.ts"; import type { InputFileFormat, JSZipFileOptions, @@ -183,7 +183,7 @@ export class JSZip { const b: Uint8Array = await this.generateAsync({ type: "uint8array" }); return await Deno.writeFile(path, b); } - + /** * Unzip a JSZip asynchronously to a directory * @@ -192,16 +192,23 @@ export class JSZip { */ async unzip(dir: string = "."): Promise { // FIXME optionally replace the existing folder prefix with dir. - for (const f of this) { - const ff = join(dir, f.name); - if (f.dir) { - // hopefully the directory is prior to any files inside it! - await Deno.mkdir(ff, { recursive: true }); - continue; - } - const content = await f.async("uint8array"); - // TODO pass WriteFileOptions e.g. mode - await Deno.writeFile(ff, content); + const createdDirs = new Set(); + + for (const fileEntry of this) { + const filePath = resolve(dir, fileEntry.name); + + const dirPath = fileEntry.dir ? filePath : dirname(filePath); + + if (!createdDirs.has(dirPath)) { + await Deno.mkdir(dirPath, { recursive: true }); + createdDirs.add(dirPath); + } + + if (!fileEntry.dir) { + const content = await fileEntry.async("uint8array"); + // TODO pass WriteFileOptions e.g. mode + await Deno.writeFile(filePath, content); + } } } From fd7453facdaf082721c5ce5f89fde14f2349871c Mon Sep 17 00:00:00 2001 From: leopf Date: Sat, 3 Jul 2021 20:11:40 +0200 Subject: [PATCH 3/4] option for replacing backwards slashes --- mod.ts | 11 +++++++---- types.ts | 4 ++++ 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/mod.ts b/mod.ts index 5109ee8..19ce118 100644 --- a/mod.ts +++ b/mod.ts @@ -1,9 +1,9 @@ import _JSZip from "https://dev.jspm.io/jszip@3.5.0"; import { WalkOptions, walk } from "https://deno.land/std@0.99.0/fs/walk.ts"; -import { SEP, join, resolve, dirname } from "https://deno.land/std@0.99.0/path/mod.ts"; +import { SEP, resolve, dirname } from "https://deno.land/std@0.99.0/path/mod.ts"; import type { InputFileFormat, - JSZipFileOptions, + JSZipAddFileOptions, JSZipGeneratorOptions, JSZipLoadOptions, JSZipObject, @@ -107,10 +107,13 @@ export class JSZip { addFile( path: string, content?: string | Uint8Array, - options?: JSZipFileOptions, + options?: JSZipAddFileOptions, ): JSZipObject { + const replaceBackslashes = options?.replaceBackslashes === undefined || options.replaceBackslashes; + const finalPath = replaceBackslashes ? path.replaceAll("\\", "/") : path; + // @ts-ignores - const f = this._z.file(path, content, options); + const f = this._z.file(finalPath, content, options); return f as JSZipObject; } diff --git a/types.ts b/types.ts index ca685ae..4099bd2 100644 --- a/types.ts +++ b/types.ts @@ -58,6 +58,10 @@ export interface JSZipFileOptions { unixPermissions?: number | string | null; } +export interface JSZipAddFileOptions extends JSZipFileOptions { + replaceBackslashes?: boolean; +} + export interface JSZipGeneratorOptions { compression?: Compression; compressionOptions?: null | { From 7e21f5b1c2eb189ed0cb810daf0c1b15454ec689 Mon Sep 17 00:00:00 2001 From: leopf Date: Sun, 4 Jul 2021 16:09:26 +0200 Subject: [PATCH 4/4] fixed exploit of unzip method, using rel paths --- mod.ts | 34 +++++++++++++++++++--------------- test.ts | 34 +++++++++++++++++++++++++++++++++- 2 files changed, 52 insertions(+), 16 deletions(-) diff --git a/mod.ts b/mod.ts index 19ce118..9f6855f 100644 --- a/mod.ts +++ b/mod.ts @@ -1,6 +1,6 @@ -import _JSZip from "https://dev.jspm.io/jszip@3.5.0"; +import _JSZip from "https://dev.jspm.io/jszip@3.5.0"; import { WalkOptions, walk } from "https://deno.land/std@0.99.0/fs/walk.ts"; -import { SEP, resolve, dirname } from "https://deno.land/std@0.99.0/path/mod.ts"; +import { SEP, resolve, dirname, globToRegExp } from "https://deno.land/std@0.99.0/path/mod.ts"; import type { InputFileFormat, JSZipAddFileOptions, @@ -109,7 +109,7 @@ export class JSZip { content?: string | Uint8Array, options?: JSZipAddFileOptions, ): JSZipObject { - const replaceBackslashes = options?.replaceBackslashes === undefined || options.replaceBackslashes; + const replaceBackslashes = options?.replaceBackslashes === undefined || options.replaceBackslashes; const finalPath = replaceBackslashes ? path.replaceAll("\\", "/") : path; // @ts-ignores @@ -186,7 +186,7 @@ export class JSZip { const b: Uint8Array = await this.generateAsync({ type: "uint8array" }); return await Deno.writeFile(path, b); } - + /** * Unzip a JSZip asynchronously to a directory * @@ -196,22 +196,26 @@ export class JSZip { async unzip(dir: string = "."): Promise { // FIXME optionally replace the existing folder prefix with dir. const createdDirs = new Set(); + const allowedFileLocRegex = globToRegExp(resolve(dir, "**")); for (const fileEntry of this) { - const filePath = resolve(dir, fileEntry.name); + const filePath = resolve(dir, fileEntry.name); + if (!allowedFileLocRegex.test(filePath)) { + throw new Error("Not allowed!"); + } - const dirPath = fileEntry.dir ? filePath : dirname(filePath); + const dirPath = fileEntry.dir ? filePath : dirname(filePath); - if (!createdDirs.has(dirPath)) { - await Deno.mkdir(dirPath, { recursive: true }); - createdDirs.add(dirPath); - } + if (!createdDirs.has(dirPath)) { + await Deno.mkdir(dirPath, { recursive: true }); + createdDirs.add(dirPath); + } - if (!fileEntry.dir) { - const content = await fileEntry.async("uint8array"); - // TODO pass WriteFileOptions e.g. mode - await Deno.writeFile(filePath, content); - } + if (!fileEntry.dir) { + const content = await fileEntry.async("uint8array"); + // TODO pass WriteFileOptions e.g. mode + await Deno.writeFile(filePath, content); + } } } diff --git a/test.ts b/test.ts index 2d8c835..794deb9 100644 --- a/test.ts +++ b/test.ts @@ -1,7 +1,8 @@ import { decode, encode } from "https://deno.land/std@0.74.0/encoding/utf8.ts"; import { join } from "https://deno.land/std@0.74.0/path/mod.ts"; -import { assertEquals } from "https://deno.land/std@0.74.0/testing/asserts.ts"; +import { assert, assertEquals, assertThrowsAsync } from "https://deno.land/std@0.74.0/testing/asserts.ts"; import { JSZip, readZip, zipDir } from "./mod.ts"; +import { exists } from "https://deno.land/std@0.100.0/fs/mod.ts"; // FIXME use tmp directory and clean up. async function exampleZip(path: string, createDirectories = true) { @@ -20,6 +21,19 @@ async function exampleZip(path: string, createDirectories = true) { await zip.writeZip(path); } +// Used for testing path exploits +async function pathExploitExampleZip() { + const zip = new JSZip(); + zip.addFile("../Hello.txt", "Hello World\n"); + + const tempFileName = await Deno.makeTempFile({ + suffix: ".zip" + }); + + await zip.writeZip(tempFileName); + return tempFileName; +} + async function fromDir(dir: string, f: () => Promise) { const cwd = Deno.cwd(); Deno.chdir(dir); @@ -94,4 +108,22 @@ Deno.test("unzip without dir", async () => { const smile = await Deno.readFile(join(dir, "images", "smile.gif")); assertEquals("", decode(smile)); +}); + +Deno.test("unzip exploit test", async () => { + const dir = await Deno.makeTempDir(); + const unpackDir = join(dir, "unpack"); + await Deno.mkdir(unpackDir); + + const zipFile = await pathExploitExampleZip(); + const z = await readZip(zipFile); + + await Deno.remove(zipFile); + + assertThrowsAsync(async () => await z.unzip(unpackDir)); + assert(!(await exists(join(dir, "Hello.txt")))); + + await Deno.remove(dir, { + recursive: true + }); }); \ No newline at end of file