Bug 18950 – Std.zip vulnerable to arbitrary file write

Created attachment 1702 POC ZIP file to test path traversal on a POSIX system Researchers from Snyk have recently disclosed a wide-spread vulnerability they call "Zip Slip" which affects many libraries in many languages. The original blog here: https://snyk.io/research/zip-slip-vulnerability The issue is two-fold: - Many ZIP libraries do not check for path traversal in file names (things like ../../../../../../../tmp/test for example. - Many applications using those libraries decompress the files to the disk without checking the names either. Zip files typically cannot contain those names normally, but they can be easily hand-crafted. This allows an attacker to leverage an application's use of std.zip to create or overwrite arbitrary files using the application's rights. This typically results in arbitrary code execution. Phobos std.zip authorizes such path traversal by default and doesn't provide any facility to do the right thing easily. Arguably this is a consummer issue: it is possible to use Phobos right by checking the name before writting the file, but requiring conciousness of the issue is asking a lot on the user. As Snyk's research shows, most vulnerable applications they found were written in Java and they attribute that to the lack of primitives doing the right thing in Java libraries. To demonstrate the issue here is a test script written at 99% using the example from std.zip that is the baseline for a good use of the library. In attachment you will find a zip file that will try to create an empty file to ../../../../../../../../../../tmp/not-good. ```D import std.digest.crc; import std.file; import std.stdio; import std.zip; void main(string[] args) { // read a zip file into memory auto zip = new ZipArchive(read(args[1])); writeln("Archive: ", args[1]); writefln("%-10s %-8s Name", "Length", "CRC-32"); // iterate over all zip members foreach (name, am; zip.directory) { // print some data about each member writefln("%10s %08x %s", am.expandedSize, am.crc32, name); assert(am.expandedData.length == 0); // decompress the archive member zip.expand(am); assert(am.expandedData.length == am.expandedSize); std.file.write(name, am.expandedData); } } ``` ```SH sh-4.4$ dmd --version DMD64 D Compiler v2.080.0-dirty Copyright (C) 1999-2018 by The D Language Foundation, All Rights Reserved written by Walter Bright sh-4.4$ ls /tmp/not-good ls: cannot access '/tmp/not-good': No such file or directory sh-4.4$ ls test.d test.zip sh-4.4$ rdmd test.d test.zip Archive: test.zip Length CRC-32 Name 0 00000000 ../../../../../../../../../../tmp/not-good sh-4.4$ ls /tmp/not-good /tmp/not-good ``` I think std.zip should do at least one of the following: - Refuse files with names resulting in path traversal by default (a flag to enable that behaviour could be used if projects actually use that, but it seems dubious); - Provide a function to decompress a file to the disk and not in memory which also checks for path traversal.

I'll keep 20290, because there is a superset of issues concerning file names listed. *** This issue has been marked as a duplicate of issue 20290 ***