CVE-2019-13232 mitigation triggers test_dir_with_symlinks

[mmahut]

Filling of CVE-2019-13232 introduced a check against a zipbomb and many (Debian, NixOS) distributions patched against it.

It seems that one of the test of zip-archive is using an archive that triggers this check and the test fails. Please see a test failed in NixOS https://logs.nix.ci/?key=nixos/nixpkgs.64909&attempt_id=cc70c8f9-1d57-4b64-8073-42691767eeda

More information about the attach https://www.bamsoftware.com/hacks/zipbomb/

And the patch applied can be found at madler/unzip@47b3cea

[jgm]

Thanks for reporting. Does this mean a zip file can no longer contain both a file and a symbolic link to that file? That is what is being tested in the test case in question. If so, this seems a misguided restriction! Surely people would link to zip up directories containing symlinks. If not, I'd like to understand how to adjust the symlink test so it doesn't trigger this check.

@tmspzz since you added support for symlinks, maybe you have some insight on this? Of course, we can always change this test so it doesn't use symlinks. But maybe there's a better way to handle symlinks that wouldn't trigger this test? (Unfortunately, my debian stable zip doesn't seem to include this test, so I can't check this myself.)

[mmahut]

Does this mean a zip file can no longer contain both a file and a symbolic link to that file? That is what is being tested in the test case in question. If so, this seems a misguided restriction! Surely people would link to zip up directories containing symlinks. If not, I'd like to understand how to adjust the symlink test so it doesn't trigger this check.

If I understand it correctly, just in case of overlapping files, not recursive files. Please see https://www.bamsoftware.com/hacks/zipbomb/

[tmspzz]

I added support for symlinks to be able to replicate the structure of Apple's framework bundles.

I believe the problem with that file is described in the article at "The first insight: overlapping files"

The failing zip has this structure:

   creating: test-zip-archive.-323ba3479951f917/test_dir_with_symlinks4/
  inflating: test-zip-archive.-323ba3479951f917/test_dir_with_symlinks4/link_to_file  -> /build/zip-archive-0.4.1/./test-zip-archive.-323ba3479951f917/test_dir_with_symlinks4/1/file.txt
   creating: test-zip-archive.-323ba3479951f917/test_dir_with_symlinks4/1/
 extracting: test-zip-archive.-323ba3479951f917/test_dir_with_symlinks4/1/file.txt

So there is 1 file in the, 1 symlink and a LFH with a small kernel

Clearly though, there is 1 file. So hardly a zip bomb to my understanding

[jgm]

@tmpszz Do you think there's anything we could be doing differently in our handling of symlinks that would avoid the problem, or is it simply impossible to have an archive in which one file is a symlink to another file in the same archive? I'm sorry, I don't understand all the details in that article, and don't have time to study it now.

[tmspzz]

@jgm I don't think it's necessary related to symlinks.

The problem might be, in my understanding, in the way the multiple Local File Header (LFH) and the Central directory header (CDH) are encoded to disk.

I cannot confirm that this is what is actually happening but to summarize for the problem is:

1. Craft a LFH with a special signature tell the extractor to consider all the bytes after the LFH as uncompressed bytes that should be copied to disk as is
2. chain N of these LFH after one another where LFH0 will have length equal to the size of all the next headers, same for LFH1 (minus the length of LFH0) and so on
3. Make LFHN a LFH containing actual data
4. Each LFH is now of size position_of_the_header*size_of_the_last_LFH

Details might not be correct but that is the attack vector