feat: safe zip reader (VF-000) (#71)

<!-- You can erase any parts of this template not applicable to your Pull Request. -->

**Fixes or implements VF-000**

### Brief description. What is this change?

Naively reading from a zip file is dangerous as it can potentially crash a system with a [zip bomb](https://en.wikipedia.org/wiki/Zip_bomb).

This provides an interface to safely read all files from a zip in a recursive manner by providing checks for number of files and total size read.

Co-authored-by: Tyler Stewart <[email protected]>

Author: trs

package.json

@@ -12,30 +12,29 @@
     }
   },
   "dependencies": {
-    "@types/chai": "^4.2.16",
-    "@types/ioredis": "^4.26.4",
-    "@types/lodash": "^4.14.168",
-    "@types/luxon": "^1.26.4",
-    "@types/sinon": "^10.0.0",
     "@voiceflow/logger": "1.6.1",
     "@voiceflow/verror": "^1.1.0",
-    "chai": "^4.3.4",
     "dotenv": "^10.0.0",
     "http-errors": "^2.0.0",
     "http-status": "^1.4.2",
     "lodash": "^4.17.11",
     "luxon": "^1.21.3",
-    "rate-limiter-flexible": "^2.2.2",
-    "sinon": "^10.0.0"
+    "rate-limiter-flexible": "^2.2.2"
   },
   "devDependencies": {
     "@commitlint/cli": "16.1.0",
     "@istanbuljs/nyc-config-typescript": "1.0.2",
     "@types/axios": "^0.14.0",
+    "@types/chai": "^4.2.16",
     "@types/chai-as-promised": "^7.1.4",
     "@types/express": "^4.17.13",
     "@types/http-errors": "^1.8.2",
+    "@types/ioredis": "^4.26.4",
+    "@types/lodash": "^4.14.168",
+    "@types/luxon": "^1.26.4",
     "@types/mocha": "9.1.0",
+    "@types/node": "^17.0.23",
+    "@types/sinon": "^10.0.0",
     "@types/supertest": "^2.0.11",
     "@voiceflow/commitlint-config": "2.0.0",
     "@voiceflow/common": "7.22.0",
@@ -44,6 +43,7 @@
     "@voiceflow/prettier-config": "1.0.6",
     "@voiceflow/tsconfig": "1.2.5",
     "@zerollup/ts-transform-paths": "^1.7.18",
+    "chai": "^4.3.4",
     "chai-as-promised": "^7.1.1",
     "commitizen": "4.2.4",
     "cz-conventional-changelog": "^3.3.0",
@@ -54,11 +54,13 @@
     "express-validator": "^6.10.1",
     "fixpack": "^4.0.0",
     "husky": "^4.3.8",
+    "jszip": "^3.7.1",
     "lint-staged": "12.2.2",
     "mocha": "9.1.4",
     "nyc": "^15.1.0",
     "prettier": "^2.2.1",
     "rimraf": "^3.0.2",
+    "sinon": "^10.0.0",
     "supertest": "6.2.2",
     "ts-mocha": "9.0.2",
     "ttypescript": "1.5.13",
@@ -76,7 +78,8 @@
   "peerDependencies": {
     "@types/express": "^4.17.13",
     "@voiceflow/common": "^7.2.0",
-    "express-validator": "^6.3.0"
+    "express-validator": "^6.3.0",
+    "jszip": "^3.7.1"
   },
   "prettier": "@voiceflow/prettier-config",
   "repository": {

src/common/index.ts

@@ -0,0 +1 @@
+export * from './zip';

src/common/zip/guard.ts

@@ -0,0 +1,6 @@
+const ZIP_MAGIC_BYTES = new Uint8Array([0x50, 0x4b, 0x03, 0x04]);
+
+export const isZipFile = (content: Uint8Array) => {
+  if (content.length < ZIP_MAGIC_BYTES.length) return false;
+  return ZIP_MAGIC_BYTES.every((byte, i) => content[i] === byte);
+};

src/common/zip/index.ts

@@ -0,0 +1,2 @@
+export * from './guard';
+export * from './reader';

src/common/zip/reader.ts

@@ -0,0 +1,90 @@
+import JSZip from 'jszip';
+
+import { isZipFile } from './guard';
+
+const DEFAULT_MAX_FILE_COUNT = 10000;
+const DEFAULT_MAX_UNZIP_SIZE_BYTES = 1e9; // 1GB
+const DEFAULT_MAX_ZIP_RECURSION_DEPTH = Infinity;
+
+/**
+ * Provides an interface to safely read all files from a zip.
+ *
+ * Ensure `jszip` is installed as a dependency to use.
+ */
+export interface ZipReaderConfig {
+  /**
+   * Provides protection from zip bombs by only reading up to this many files.
+   * Will throw an error if the zip file exceeds this count.
+   */
+  maxFileCount: number;
+  /**
+   * Provides protection from zip bombs by only reading up to this many bytes total.
+   * Will throw an error if the total bytes exceeds this count.
+   */
+  maxUnzipSizeBytes: number;
+  /**
+   * If the zip file contains zip files, they will be extracted up to this many deep.
+   * If the depth exceeds this number, then these deeper zip files will be returned instead of extracted.
+   *
+   * `0` means do not recurse.
+   */
+  maxZipRecursionDepth: number;
+}
+
+export interface ZipEntry {
+  name: string;
+  content: Uint8Array;
+}
+
+export class ZipReader {
+  private readonly config: ZipReaderConfig;
+
+  public constructor(private readonly zip: JSZip, config: Partial<ZipReaderConfig> = {}) {
+    this.config = {
+      maxFileCount: DEFAULT_MAX_FILE_COUNT,
+      maxUnzipSizeBytes: DEFAULT_MAX_UNZIP_SIZE_BYTES,
+      maxZipRecursionDepth: DEFAULT_MAX_ZIP_RECURSION_DEPTH,
+      ...config,
+    };
+  }
+
+  public async *getFiles(): AsyncGenerator<ZipEntry> {
+    yield* this.getFilesRecursively(this.zip, this.config.maxZipRecursionDepth);
+  }
+
+  private async *getFilesRecursively(zip: JSZip, maxDepth: number, currentDepth = 0): AsyncGenerator<ZipEntry> {
+    const objects = zip.filter(() => true);
+
+    let fileCount = 0;
+    let totalSize = 0;
+
+    // eslint-disable-next-line no-restricted-syntax
+    for (const obj of objects) {
+      if (obj.dir) continue;
+
+      fileCount += 1;
+      if (fileCount > this.config.maxFileCount) {
+        throw new Error(`File count exceeded maximum (${this.config.maxFileCount})`);
+      }
+
+      // eslint-disable-next-line no-await-in-loop
+      const content = await obj.async('uint8array');
+
+      totalSize += content.byteLength;
+      if (totalSize > this.config.maxUnzipSizeBytes) {
+        throw new Error(`Total file size exceeded maximum (${this.config.maxUnzipSizeBytes} bytes)`);
+      }
+
+      if (currentDepth < maxDepth && isZipFile(content)) {
+        // eslint-disable-next-line no-await-in-loop
+        const zip = await JSZip.loadAsync(content);
+        yield* this.getFilesRecursively(zip, maxDepth, currentDepth + 1);
+      } else {
+        yield {
+          name: obj.name,
+          content,
+        };
+      }
+    }
+  }
+}

src/fixtureGenerator.ts

@@ -1,7 +1,9 @@
 /* eslint-disable no-param-reassign,@typescript-eslint/no-empty-function */
+// eslint-disable-next-line import/no-extraneous-dependencies
 import { expect } from 'chai';
 import { Request, Response } from 'express';
 import _ from 'lodash';
+// eslint-disable-next-line import/no-extraneous-dependencies
 import sinon from 'sinon';
 
 import { ServiceManager } from './types';

src/index.ts

@@ -1,4 +1,5 @@
 export * from './clients';
+export * as Common from './common';
 export * from './env';
 export { default as FixtureGenerator } from './fixtureGenerator';
 export * from './middlewares';

tests/common/zip/reader.unit.ts

@@ -0,0 +1,62 @@
+import { expect } from 'chai';
+import JSZip from 'jszip';
+
+import { ZipReader } from '../../../src/common/zip';
+
+describe('ZipReader', () => {
+  let zip: JSZip;
+
+  const asyncArrayFrom = async <T>(iterator: AsyncIterableIterator<T>): Promise<T[]> => {
+    const array: T[] = [];
+    // eslint-disable-next-line no-restricted-syntax
+    for await (const item of iterator) {
+      array.push(item);
+    }
+    return array;
+  };
+
+  beforeEach(() => {
+    zip = new JSZip();
+    zip.file('file.json', JSON.stringify({ hello: 'world' }));
+    zip.file('nested/file.zip', new JSZip().file('inside.json', JSON.stringify({ a: 1 })).generateNodeStream());
+  });
+
+  afterEach(() => {
+    (zip as any) = undefined;
+  });
+
+  it('deeply reads all files, unzips internal zips', async () => {
+    const reader = new ZipReader(zip);
+    const files = await asyncArrayFrom(reader.getFiles());
+
+    expect(files.length).to.equal(2);
+    expect(files[0].name).to.equal('file.json');
+    expect(new TextDecoder().decode(files[0].content)).to.equal(JSON.stringify({ hello: 'world' }));
+
+    expect(files[1].name).to.equal('inside.json');
+    expect(new TextDecoder().decode(files[1].content)).to.equal(JSON.stringify({ a: 1 }));
+  });
+
+  it('throws if total size exceeds limit', async () => {
+    const reader = new ZipReader(zip, {
+      maxUnzipSizeBytes: 1,
+    });
+    expect(asyncArrayFrom(reader.getFiles())).to.eventually.throw(/^Total file size exceeded maximum/);
+  });
+
+  it('throws if total file count exceeds limit', async () => {
+    const reader = new ZipReader(zip, {
+      maxFileCount: 1,
+    });
+    expect(asyncArrayFrom(reader.getFiles())).to.eventually.throw(/^File count exceeded maximum/);
+  });
+
+  it('limits recursion to limit', async () => {
+    const reader = new ZipReader(zip, { maxZipRecursionDepth: 0 });
+    const files = await asyncArrayFrom(reader.getFiles());
+
+    expect(files.length).to.equal(2);
+    expect(files[0].name).to.equal('file.json');
+    expect(files[1].name).to.equal('nested/file.zip');
+  });
+});