ZIP with file "../Hello.txt" creates "Hello.txt" file in super directory of unpack target

[leopf]

While playing around with the unzip method I found a vulnerability which allows creating files in arbitrary directories using relative paths, when adding files.

Lets say I want to unpack a ZIP file into data/unpack:
I would call:

await zip.unzip("data/unpack");

The zip I want to unpack was created in the following way:

const zip = new JSZip();
zip.addFile("../Hello.txt", "Hello World!");
// ...other files

After calling unpack, the directory structure would look like this:

.
data
│   Hello.txt    
│
└───unpack
│   │  ...

This is obviously a problem. If I had a file called "server.ts" in the root directory, it would be possible to strategically replace this with a fraudulent file.

The examples don't show that this is possible if the zip is saved and loaded from the disk before unpacking it, but I tested this and it works. Common zip tools throw an error in this case (some ignore the file and some move them to the root).

A possible solution to my previously suggested update to the unzip method would look like this:

async unzip(dir: string = "."): Promise<void> {
  // FIXME optionally replace the existing folder prefix with dir.
  const createdDirs = new Set<string>();
  const absDir = resolve(dir);

  for (const fileEntry of this) {
    const filePath = resolve(dir, fileEntry.name);

    const commonRootPath = resolve(join(common([absDir, filePath]), "/"));
    if (commonRootPath !== absDir || commonRootPath === filePath) {
      throw new Error("Not allowed!");
    }

    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);
    }
  }
}

This update would not overwrite anything in the super directory and throw an error, but the files that were valid would still be created, which might not be wanted. A solution for this would be a complete validation of all files and folders before executing any unzip operation. I have not yet tested this solution fully, some more testing might be required, especially when attempting to overwrite the unpack directory target with a file having the equal name, which could cause crashes and be used as a DoS attack.

While this vulnerability exists in my suggested, updated unzip method, it also exists in the old one, which is the reason for this second issue.

Edit: spelling and formatting

[hayd]

yikes! Another great find