Fix a potential ZIP slip (#9098)

* Fix a potential ZIP slip

* Fixing tests and zip slip beavhior

* Format

Author: joehan

CHANGELOG.md

@@ -1,3 +1,4 @@
 - `firebase emulator:start` use a default project `demo-no-project` if no project can be found. (#9072)
 - `firebase init dataconnect` also supports bootstrapping flutter template. (#9084)
+- Fixed a vulnerability in `unzip` util where files could be written outside of the expected output directory.
 - `firebase init dataconnect` confirms Cloud SQL provisioning. (#9095)

src/test/fixtures/zip-files/index.ts

@@ -4,15 +4,16 @@ const zipFixturesDir = __dirname;
 const testDataDir = join(zipFixturesDir, "node-unzipper-testData");
 
 export const ZIP_CASES = [
-  "compressed-cp866",
-  "compressed-directory-entry",
-  "compressed-flags-set",
-  "compressed-standard",
-  "uncompressed",
-  "zip-slip",
-  "zip64",
-].map((name) => ({
+  { name: "compressed-cp866" },
+  { name: "compressed-directory-entry" },
+  { name: "compressed-flags-set" },
+  { name: "compressed-standard" },
+  { name: "uncompressed" },
+  { name: "zip-slip", wantErr: "a path outside of" },
+  { name: "zip64" },
+].map(({ name, wantErr }) => ({
   name,
   archivePath: join(testDataDir, name, "archive.zip"),
   inflatedDir: join(testDataDir, name, "inflated"),
+  wantErr,
 }));

src/unzip.spec.ts

@@ -16,14 +16,21 @@ describe("unzip", () => {
     await fs.promises.rm(tempDir, { recursive: true });
   });
 
-  for (const { name, archivePath, inflatedDir } of ZIP_CASES) {
-    it(`should unzip a zip file with ${name} case`, async () => {
-      const unzipPath = path.join(tempDir, name);
-      await unzip(archivePath, unzipPath);
+  for (const { name, archivePath, inflatedDir, wantErr } of ZIP_CASES) {
+    if (!wantErr) {
+      it(`should unzip a zip file with ${name} case`, async () => {
+        const unzipPath = path.join(tempDir, name);
+        await unzip(archivePath, unzipPath);
 
-      const expectedSize = await calculateFolderSize(inflatedDir);
-      expect(await calculateFolderSize(unzipPath)).to.eql(expectedSize);
-    });
+        const expectedSize = await calculateFolderSize(inflatedDir);
+        expect(await calculateFolderSize(unzipPath)).to.eql(expectedSize);
+      });
+    } else {
+      it(`should throw "${wantErr}" when reading a zip file with ${name} case`, async () => {
+        const unzipPath = path.join(tempDir, name);
+        expect(unzip(archivePath, unzipPath)).to.eventually.be.rejectedWith(wantErr);
+      });
+    }
   }
 });
 

src/unzip.ts

@@ -85,6 +85,12 @@ const extractEntriesFromBuffer = async (data: Buffer, outputDir: string): Promis
     entry.fileName = entry.fileName.replace(/\//g, path.sep);
 
     const outputFilePath = path.normalize(path.join(outputDir, entry.fileName));
+    // Don't allow traversal outside of outputDir
+    if (!isChildDir(outputDir, outputFilePath)) {
+      throw new FirebaseError(
+        `ZIP contained an entry for ${outputFilePath},  a path outside of ${outputDir}`,
+      );
+    }
 
     logger.debug(`[unzip] Processing entry: ${entry.fileName}`);
     if (entry.fileName.endsWith(path.sep)) {
@@ -117,6 +123,20 @@ const extractEntriesFromBuffer = async (data: Buffer, outputDir: string): Promis
   }
 };
 
+function isChildDir(parentDir: string, potentialChild: string): boolean {
+  try {
+    // 1. Resolve and normalize both paths to absolute paths
+    const resolvedParent = path.resolve(parentDir);
+    const resolvedChild = path.resolve(potentialChild);
+    // The child path must start with the parent path and not be the same path.
+    return resolvedChild.startsWith(resolvedParent) && resolvedChild !== resolvedParent;
+  } catch (error) {
+    // If either path does not exist, an error will be thrown.
+    // In this case, the potential child cannot be a subdirectory.
+    return false;
+  }
+}
+
 export const unzip = async (inputPath: string, outputDir: string): Promise<void> => {
   const data = await fs.promises.readFile(inputPath);
   await extractEntriesFromBuffer(data, outputDir);