Skip to content

reject directory entries in the noalloc entry readers#441

Merged
kuba-- merged 2 commits into
kuba--:masterfrom
jmestwa-coder:noalloc-read-dir-entry
Jul 15, 2026
Merged

reject directory entries in the noalloc entry readers#441
kuba-- merged 2 commits into
kuba--:masterfrom
jmestwa-coder:noalloc-read-dir-entry

Conversation

@jmestwa-coder

Copy link
Copy Markdown
Contributor

zip_entry_read and zip_entry_fread reject a directory entry, but the two no-alloc readers don't:

  • zip_entry_noallocread returns the declared uncomp_size while miniz's extractor reports success without ever writing buf or checking bufsize (it short-circuits directory entries first), so a crafted "a/" deflate entry with a large uncomp_size reports far more bytes than were written into the caller's fixed buffer
  • zip_entry_noallocreadwithoffset takes the same entry through extract_to_heap and copies a window out of the unwritten heap block, leaking uninitialized memory
  • added the is_file_a_directory guard both readers already use, returning ZIP_EINVENTTYPE, plus a regression test

@kuba--

kuba-- commented Jul 14, 2026

Copy link
Copy Markdown
Owner

zip_entry_noallocreadwithoffset() checks the offset before checking whether the entry is a directory:

if (offset >= (size_t)zip->entry.uncomp_size)
  return ZIP_EINVAL;
/* Later: */
if (mz_zip_reader_is_file_a_directory(...))
  return ZIP_EINVENTTYPE;

Therefore, for a directory:

offset < declared uncomp_size → ZIP_EINVENTTYPE
offset >= declared uncomp_size → ZIP_EINVAL

Moving the directory check earlier would make every directory call consistently return ZIP_EINVENTTYPE.

@jmestwa-coder

Copy link
Copy Markdown
Contributor Author

good point, moved the directory check ahead of the offset check so a directory entry returns ZIP_EINVENTTYPE regardless of offset. added an assertion with offset past the declared size to lock that in.

@kuba--
kuba-- merged commit 322652c into kuba--:master Jul 15, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants