Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Heap-buffer-overflow in mz_os.c:71 mz_path_has_slash #739

Closed
Akane0721 opened this issue Nov 8, 2023 · 13 comments
Closed

Heap-buffer-overflow in mz_os.c:71 mz_path_has_slash #739

Akane0721 opened this issue Nov 8, 2023 · 13 comments
Labels
bug Bug fixed Issue or bug has been fixed security Security issue

Comments

@Akane0721
Copy link

Description

heap-buffer-overflow (/minizip-ng/build/minizip+0x8d5a) in mz_path_has_slash

Version

$ ./minizip -h
minizip-ng 4.0.2 - https://github.com/zlib-ng/minizip-ng
---------------------------------------------------
-h 
Usage: minizip [-x][-d dir|-l|-e][-o][-f][-y][-c cp][-a][-0 to -9][-b|-m|-t][-k 512][-p pwd][-s] file.zip [files]

  -x  Extract files
  -l  List files
  -d  Destination directory
  -e  Erase files
  -o  Overwrite existing files
  -c  File names use cp437 encoding (or specified codepage)
  -a  Append to existing zip file
  -i  Include full path of files
  -f  Follow symbolic links
  -y  Store symbolic links
  -v  Verbose info
  -0  Store only
  -1  Compress faster
  -9  Compress better
  -k  Disk size in KB
  -z  Zip central directory
  -p  Encryption password
  -s  AES encryption
  -b  BZIP2 compression
  -m  LZMA compression
  -n  XZ compression
  -t  ZSTD compression

Replay

git clone https://github.com/zlib-ng/minizip-ng.git
cd minizip-ng/
export AFL_USE_ASAN=1
CC="gcc -fsanitize=address" CXX="g++ -fsanitize=address" cmake -S . -B build -D MZ_BUILD_TESTS=ON
cmake --build build
./minizip -x -o poc0

ASAN

-x -o /root/poc/minizip/poc0 
Archive /root/poc/minizip/poc0
Extracting .\
=================================================================
==1735959==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6020000000cf at pc 0x55555555cd5b bp 0x7fffffffdd20 sp 0x7fffffffdd10
READ of size 1 at 0x6020000000cf thread T0
    #0 0x55555555cd5a in mz_path_has_slash (/root/test/minizip-ng/build/minizip+0x8d5a)
    #1 0x555555572ac1 in mz_zip_reader_entry_save_file (/root/test/minizip-ng/build/minizip+0x1eac1)
    #2 0x555555573ceb in mz_zip_reader_save_all (/root/test/minizip-ng/build/minizip+0x1fceb)
    #3 0x55555555af70 in minizip_extract (/root/test/minizip-ng/build/minizip+0x6f70)
    #4 0x55555555c7ec in main (/root/test/minizip-ng/build/minizip+0x87ec)
    #5 0x7ffff73a9082 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24082)
    #6 0x555555558aed in _start (/root/test/minizip-ng/build/minizip+0x4aed)

0x6020000000cf is located 1 bytes to the left of 1-byte region [0x6020000000d0,0x6020000000d1)
allocated by thread T0 here:
    #0 0x7ffff76a0a06 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:153
    #1 0x555555572749 in mz_zip_reader_entry_save_file (/root/test/minizip-ng/build/minizip+0x1e749)
    #2 0x555555573ceb in mz_zip_reader_save_all (/root/test/minizip-ng/build/minizip+0x1fceb)
    #3 0x55555555af70 in minizip_extract (/root/test/minizip-ng/build/minizip+0x6f70)
    #4 0x55555555c7ec in main (/root/test/minizip-ng/build/minizip+0x87ec)
    #5 0x7ffff73a9082 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24082)

SUMMARY: AddressSanitizer: heap-buffer-overflow (/root/test/minizip-ng/build/minizip+0x8d5a) in mz_path_has_slash
Shadow bytes around the buggy address:
  0x0c047fff7fc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7fd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7fe0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7ff0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff8000: fa fa fd fd fa fa fd fd fa fa 00 07 fa fa 00 01
=>0x0c047fff8010: fa fa 00 01 fa fa 00 01 fa[fa]01 fa fa fa 01 fa
  0x0c047fff8020: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8030: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8040: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8050: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8060: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==1735959==ABORTING

POC

https://github.com/Akane0721/POC/blob/f37d805631e0a15bea1f15b6e1edfb3246a2e0fc/minizip-ng/poc0

Environment

Ubuntu 20.04
Clang 10.0.0
gcc 9.4.0
@nmoinvaz nmoinvaz added the help wanted Need outside help label Nov 8, 2023
@pmqs
Copy link
Contributor

pmqs commented Nov 8, 2023

Where did poc0 come from?

minizip thinks it is badly formed

$ ./minizip -l poc0
minizip-ng 4.0.1 - https://github.com/zlib-ng/minizip-ng
---------------------------------------------------
-l poc0 
      Packed     Unpacked Ratio Method   Attribs Date     Time  CRC-32     Name
      ------     -------- ----- ------   ------- ----     ----  ------     ----
         141          191   73% deflate  a1a00000 11-07-14 06:22 f0c14f39   .\
Error -103 going to next entry in archive

so does unzip

$ unzip -l poc0
Archive:  poc0
  Length      Date    Time    Name
---------  ---------- -----   ----
      191  2014-11-07 06:22   .\
---------                     -------
      191                     1 file

note:  didn't find end-of-central-dir signature at end of central dir.

and 7z

$ 7z l  poc0 

7-Zip [64] 16.02 : Copyright (c) 1999-2016 Igor Pavlov : 2016-05-21
p7zip Version 16.02 (locale=en_GB.UTF-8,Utf16=on,HugeFiles=on,64 bits,2 CPUs Intel(R) Core(TM) i7-6700 CPU @ 3.40GHz (506E3),ASM,AES-NI)

Scanning the drive for archives:
1 file, 289 bytes (1 KiB)

Listing archive: poc0

--
Path = poc0
Type = zip
ERRORS:
Headers Error
WARNINGS:
Headers Error
There are data after the end of archive
Physical Size = 265
Tail Size = 24

   Date      Time    Attr         Size   Compressed  Name
------------------- ----- ------------ ------------  ------------------------
2014-11-07 05:22:56 .....          191          141  limerick
------------------- ----- ------------ ------------  ------------------------
2014-11-07 05:22:56                191          141  1 files

Warnings: 1

Errors: 1

@Akane0721
Copy link
Author

Akane0721 commented Nov 9, 2023

Where did poc0 come from?

minizip thinks it is badly formed

$ ./minizip -l poc0
minizip-ng 4.0.1 - https://github.com/zlib-ng/minizip-ng
---------------------------------------------------
-l poc0 
      Packed     Unpacked Ratio Method   Attribs Date     Time  CRC-32     Name
      ------     -------- ----- ------   ------- ----     ----  ------     ----
         141          191   73% deflate  a1a00000 11-07-14 06:22 f0c14f39   .\
Error -103 going to next entry in archive

so does unzip

$ unzip -l poc0
Archive:  poc0
  Length      Date    Time    Name
---------  ---------- -----   ----
      191  2014-11-07 06:22   .\
---------                     -------
      191                     1 file

note:  didn't find end-of-central-dir signature at end of central dir.

and 7z

$ 7z l  poc0 

7-Zip [64] 16.02 : Copyright (c) 1999-2016 Igor Pavlov : 2016-05-21
p7zip Version 16.02 (locale=en_GB.UTF-8,Utf16=on,HugeFiles=on,64 bits,2 CPUs Intel(R) Core(TM) i7-6700 CPU @ 3.40GHz (506E3),ASM,AES-NI)

Scanning the drive for archives:
1 file, 289 bytes (1 KiB)

Listing archive: poc0

--
Path = poc0
Type = zip
ERRORS:
Headers Error
WARNINGS:
Headers Error
There are data after the end of archive
Physical Size = 265
Tail Size = 24

   Date      Time    Attr         Size   Compressed  Name
------------------- ----- ------------ ------------  ------------------------
2014-11-07 05:22:56 .....          191          141  limerick
------------------- ----- ------------ ------------  ------------------------
2014-11-07 05:22:56                191          141  1 files

Warnings: 1

Errors: 1

poc0 is a malformed zip file generated by fuzzer. I used the "-x" flag when testing and it came into a heap-buffer-overflow crash. So maybe you could give a proper prompt when using "-x" to extract malformed files like poc0?

@pmqs
Copy link
Contributor

pmqs commented Nov 9, 2023

poc0 is a malformed zip file generated by fuzzer. I used the "-x" flag when testing and it came into a heap-buffer-overflow crash. So maybe you could give a proper prompt when using "-x" to extract malformed files like poc0?

Aaaah, ok.

When built without ASAN the poc0 zipfile triggers an error when you attempt to extract it

$ ./minizip -x  poc0
minizip-ng 4.0.2 - https://github.com/zlib-ng/minizip-ng
---------------------------------------------------
-x poc0 
Archive poc0
Extracting .\
Error -104 saving entries to disk poc0

That said, there is still a buffer overflow present. Let's take a look at that

FYI - the cmake build now support building with ASAN, like this.

 cmake -S . -B build -D MZ_BUILD_TESTS=ON -DMZ_SANITIZER=Address 

When I run that I get the line numbers where the problems are

 ./minizip -x poc0
minizip-ng 4.0.2 - https://github.com/zlib-ng/minizip-ng
---------------------------------------------------
-x poc0 
Archive poc0
Extracting .\
=================================================================
==49164==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60200000010f at pc 0x55686d69e58d bp 0x7ffc7a5e2d80 sp 0x7ffc7a5e2d70
READ of size 1 at 0x60200000010f thread T0
    #0 0x55686d69e58c in mz_path_has_slash /home/paul/git/minizip-ng/mz_os.c:71
    #1 0x55686d6b55e1 in mz_zip_reader_entry_save_file /home/paul/git/minizip-ng/mz_zip_rw.c:698
    #2 0x55686d6b67ef in mz_zip_reader_save_all /home/paul/git/minizip-ng/mz_zip_rw.c:893
    #3 0x55686d69c6f0 in minizip_extract /home/paul/git/minizip-ng/minizip.c:379
    #4 0x55686d69e02c in main /home/paul/git/minizip-ng/minizip.c:654
    #5 0x7f2f44c280cf in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #6 0x7f2f44c28188 in __libc_start_main_impl ../csu/libc-start.c:360
    #7 0x55686d69a264 in _start (/home/paul/git/minizip-ng/build/minizip+0x7264) (BuildId: 576262a7c293cbb21845b25bd97a13cb33d9dd27)

0x60200000010f is located 1 bytes before 1-byte region [0x602000000110,0x602000000111)
allocated by thread T0 here:
    #0 0x7f2f456de997 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:77
    #1 0x55686d6b5269 in mz_zip_reader_entry_save_file /home/paul/git/minizip-ng/mz_zip_rw.c:665
    #2 0x55686d6b67ef in mz_zip_reader_save_all /home/paul/git/minizip-ng/mz_zip_rw.c:893
    #3 0x55686d69c6f0 in minizip_extract /home/paul/git/minizip-ng/minizip.c:379
    #4 0x55686d69e02c in main /home/paul/git/minizip-ng/minizip.c:654
    #5 0x7f2f44c280cf in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58

SUMMARY: AddressSanitizer: heap-buffer-overflow /home/paul/git/minizip-ng/mz_os.c:71 in mz_path_has_slash
Shadow bytes around the buggy address:
  0x601ffffffe80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x601fffffff00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x601fffffff80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x602000000000: fa fa 05 fa fa fa 00 06 fa fa fd fd fa fa fd fd
  0x602000000080: fa fa 00 07 fa fa 00 01 fa fa 00 01 fa fa 00 01
=>0x602000000100: fa[fa]01 fa fa fa 01 fa fa fa fa fa fa fa fa fa
  0x602000000180: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x602000000200: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x602000000280: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x602000000300: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x602000000380: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==49164==ABORTING

Memory allocation in the pathwfs in mz_zip_reader_entry_save_file

int32_t mz_zip_reader_entry_save_file(void *handle, const char *path) {
    mz_zip_reader *reader = (mz_zip_reader *)handle;
    void *stream = NULL;
    uint32_t target_attrib = 0;
    int32_t err_attrib = 0;
    int32_t err = MZ_OK;
    int32_t err_cb = MZ_OK;
    size_t path_length = 0;
    char *pathwfs = NULL;
    char *directory = NULL;

    if (mz_zip_reader_is_open(reader) != MZ_OK)
        return MZ_PARAM_ERROR;
    if (!reader->file_info || !path)
        return MZ_PARAM_ERROR;

    path_length = strlen(path);

    /* Convert to forward slashes for unix which doesn't like backslashes */
    pathwfs = (char *)calloc(path_length + 1, sizeof(char));

The error is triggered in the if statement below

int32_t mz_path_has_slash(const char *path) {
    int32_t path_len = (int32_t)strlen(path);
    if (path[path_len - 1] != '\\' && path[path_len - 1] != '/')
        return MZ_EXIST_ERROR;
    return MZ_OK;
}

This needs further analysis

@nmoinvaz
Copy link
Member

Does checking for path_len > 0 help?

@pmqs
Copy link
Contributor

pmqs commented Nov 12, 2023

Does checking for path_len > 0 help?

Yes it does. The error is triggered when mz_path_has_slash is called with a zero length path.

Fix in PR #741

@pmqs
Copy link
Contributor

pmqs commented Nov 12, 2023

@Akane0721 are your fuzz test available somewhere? They don't appear to be in the fuzzing that this project uses.

@Akane0721
Copy link
Author

@Akane0721 are your fuzz test available somewhere? They don't appear to be in the fuzzing that this project uses.

https://github.com/fdu-sec/NestFuzz

@pmqs
Copy link
Contributor

pmqs commented Nov 13, 2023

@Akane0721 are your fuzz test available somewhere? They don't appear to be in the fuzzing that this project uses.

https://github.com/fdu-sec/NestFuzz

Thanks @Akane0721 but I don't see the fuzz test you used for minizip in that repo. Have I missed something? If you specific tests that use NestFuzz can you share them?

@Akane0721
Copy link
Author

@Akane0721 are your fuzz test available somewhere? They don't appear to be in the fuzzing that this project uses.

https://github.com/fdu-sec/NestFuzz

Thanks @Akane0721 but I don't see the fuzz test you used for minizip in that repo. Have I missed something? If you specific tests that use NestFuzz can you share them?

Sorry, I'm not quite clear on what you mean by "fuzz test." Are you referring to the fuzzer's output folder for minizip or the fuzzer_stats file or something?

@pmqs
Copy link
Contributor

pmqs commented Nov 13, 2023

@Akane0721 are your fuzz test available somewhere? They don't appear to be in the fuzzing that this project uses.

https://github.com/fdu-sec/NestFuzz

Thanks @Akane0721 but I don't see the fuzz test you used for minizip in that repo. Have I missed something? If you specific tests that use NestFuzz can you share them?

Sorry, I'm not quite clear on what you mean by "fuzz test." Are you referring to the fuzzer's output folder for minizip or the fuzzer_stats file or something?

No. problem. The question is how to reproduce the test you ran with minizip + NestFuzz that highlighted the issue you have reported?

@Akane0721
Copy link
Author

@Akane0721 are your fuzz test available somewhere? They don't appear to be in the fuzzing that this project uses.

https://github.com/fdu-sec/NestFuzz

Thanks @Akane0721 but I don't see the fuzz test you used for minizip in that repo. Have I missed something? If you specific tests that use NestFuzz can you share them?

Sorry, I'm not quite clear on what you mean by "fuzz test." Are you referring to the fuzzer's output folder for minizip or the fuzzer_stats file or something?

No. problem. The question is how to reproduce the test you ran with minizip + NestFuzz that highlighted the issue you have reported?

output_dir link: https://github.com/Akane0721/POC/tree/f2b5178655c1e8fa9acbc1d6ba50be0c62d36016/minizip-out-folder (only kept the non-duplicated "crashes" files), and the input seed zip file is also put there.

fuzzer command:

export AFL_USE_ASAN=1 
CC="path/to/NestFuzz/afl-gcc" CXX="path/to/NestFuzz/afl-g++" cmake -S . -B build -D MZ_BUILD_TESTS=ON
cmake --build build
path/to/NestFuzz/./afl-fuzz -m none -d -i $INPUT -o $OUTPUT -- $PROG -x -o @@

input logic processing command:

CC="path/to/NestFuzz/ipl-modeling/install/test-clang" CXX="path/to/NestFuzz/ipl-modeling/install/test-clang++" cmake -S . -B build -D MZ_BUILD_TESTS=ON
cmake --build build
python3 $ROOTDIR/fuzzer/NestFuzz/isi.py -t 300 -o $OUTPUT -l $LOG -- $PROGINFER -x -o @@

@pmqs
Copy link
Contributor

pmqs commented Nov 13, 2023

Thanks @Akane0721 !

nmoinvaz pushed a commit that referenced this issue Nov 13, 2023
@nmoinvaz nmoinvaz added fixed Issue or bug has been fixed security Security issue bug Bug and removed help wanted Need outside help labels Nov 13, 2023
@nmoinvaz
Copy link
Member

Fixed in 4.0.3. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug fixed Issue or bug has been fixed security Security issue
Projects
None yet
Development

No branches or pull requests

3 participants