Names truncated at 256 bytes

[pmqs]

While looking at #689 I noticed a number of arrays (path , utf8_name and resolved_name ) with hard-wired lengths in mz_zip_reader_save_all. The code that uses these arrays looks like it can silently truncate output filenames when the overall path being written exceeds these array lengths.

The attached zip file, test.zip, contains a patnh with two 150 character directory names. Individually they are ok, but the combined lengths will result is a path that blows the 256 byte length of the utf8_name array.

$ unzip -l test.zip 
Archive:  test.zip
  Length      Date    Time    Name
---------  ---------- -----   ----
        5  2023-04-06 14:47   base/xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy/text.txt
---------                     -------
        5                     1 file

The ultimate payload, text.txt, contains this

$ unzip -p test.zip 
abcd

Lets see what unzip extracts

$ unzip test.zip 
Archive:  test.zip
  inflating: base/xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy/text.txt  

$ find base
base
base/xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
base/xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy
base/xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy/text.txt

$ cat base/xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy/text.txt 
abcd

That looks fine.

Now try minizip. Listing the contents looks ok

$ minizip -l test.zip 
minizip-ng 3.0.7 - https://github.com/zlib-ng/minizip-ng
---------------------------------------------------
-l test.zip 
      Packed     Unpacked Ratio Method   Attribs Date     Time  CRC-32     Name
      ------     -------- ----- ------   ------- ----     ----  ------     ----
           7            5  140% deflate  81a40000 04-06-23 14:47 588aa4ac   base/xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy/text.txt

Now for an extract -- the logging looks ok

$ minizip -x test.zip 
minizip-ng 3.0.7 - https://github.com/zlib-ng/minizip-ng
---------------------------------------------------
-x test.zip 
Archive test.zip
Extracting base/xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy/text.txt

The extracted data is not correct -- it has truncated at 256 bytes

$ find base
base
base/xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
base/xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy

$ cat base/xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy 
abcd

[nmoinvaz]

If you can do a PR that would be good.

[pmqs]

Yes, it is. See https://github.com/pmqs/minizip-ng/tree/develop for a fix in progress.

…

From: ra-dave Sent: Friday, May 26, 2023 6:39 PM To: zlib-ng/minizip-ng Subject: Re: [zlib-ng/minizip-ng] Names truncated at 256 bytes (Issue #690) Is this still an issue in minizip-ng 4.0.0? BTW, Windows 11 can't even open that test.zip file: https://user-images.githubusercontent.com/24297299/241287067-5a71fecd-d869-4f82-9610-58ea775f3210.png

[pmqs]

@nmoinvaz, what, if anything, is the minimum version of C that this project supports?

In the fix for this issue I had originally written the code below to dynamically allocate space for the pathwfs and directory variables in mz_zip_reader_entry_save_file

    size_t len = strlen(path);
    char pathwfs[len+ 1];
    char directory[len) + 1];

The problem is that code relies on the Variable-length array feature from C99. It would appear that the MSVC build in the github workflow doesn't support that -- see the failure at https://github.com/pmqs/minizip-ng/actions/runs/5110532731/jobs/9186528243

Getting the MSVC build to work means using malloc. That isn't a problem, but there are 8 error exit points in mz_zip_reader_entry_save_file. Having to add two calls to free for each of them is certainly a possibility, but it really adds to the clutter.

Currently I've written a POC wrapper function to do the two calls to malloc, then call mz_zip_reader_entry_save_file and free the memory buffers when done. The has build ok with MSVC (https://github.com/pmqs/minizip-ng/actions/runs/5136441700/jobs/9243183824).

I'm leaning towards the wrapper solution if C99 isn't a possibility . Opinions welcome.

[nmoinvaz]

Yes you can just use alloca or _alloca on windows.

[pmqs]

Aaah! Didn't know about alloca. Will go with something like this then

    size_t len = strlen(path);
#if defined(_WIN32)
    char *pathwfs = alloca(len + 1);
    char *directory = alloca(len + 1);
#else
    char pathwfs[len+ 1];
    char directory[len) + 1];
#endif

[nmoinvaz]

At top of file:

#ifdef _WIN32
#  define alloca _alloca
#endif

Then ..

    char *pathwfs = alloca(len + 1);
    char *directory = alloca(len + 1);
   if (!pathwfs || !directory) 
   return err...

[pmqs]

At top of file:

#ifdef _WIN32
#  define alloca _alloca
#endif

Then ..

    char *pathwfs = alloca(len + 1);
    char *directory = alloca(len + 1);
   if (!pathwfs || !directory) 
   return err...

Almost, but not quite

[ 11%] Building C object CMakeFiles/minizip.dir/mz_zip_rw.c.o
/media/paul/Linux-Shared/base/git/minizip-ng/mz_zip_rw.c: In function ‘mz_zip_reader_entry_save_file’:
/media/paul/Linux-Shared/base/git/minizip-ng/mz_zip_rw.c:663:29: warning: implicit declaration of function ‘alloca’ [-Wimplicit-function-declaration]
  663 |     char *pathwfs = (char *)alloca(path_length + 1) ;
      |                             ^~~~~~
/media/paul/Linux-Shared/base/git/minizip-ng/mz_zip_rw.c:663:29: warning: incompatible implicit declaration of built-in function ‘alloca’ [-Wbuiltin-declaration-mismatch]

this silences the warnings

/* for alloca */

#if defined(_WIN32)
  #include <malloc.h>
#else
  #include <alloca.h>
#endif

[nmoinvaz]

Should be resolved now in the develop branch.