Skip to content

Commit 1235db3

Browse files
committed
file/archive_file_zlib: harden ZIP central-directory parser
Two related issues in the ZIP central-directory walker and filename callback, both reachable from any ZIP load (ROM scan, content load, extract-to-dir, etc): 1. zip_parse_file_iterate_step_internal() checked only that the directory entry pointer fell inside the directory block, not that a full 46-byte central-file-header plus the variable-length name, extra and comment fields actually fit. A malformed archive with a truncated trailing entry caused read_le() to read past the allocation. Reproduced under AddressSanitizer with a 40-byte directory: ERROR: AddressSanitizer: heap-buffer-overflow READ of size 1 at 2 bytes after 40-byte region Fix by checking the available entry size (a) before the header reads, and (b) before the memcpy of the filename, against the declared namelength/extralength/commentlength. 2. zip_file_decompressed() computed "name[strlen(name) - 1]" without guarding against an empty filename entry. When strlen == 0 the index wraps to SIZE_MAX and the dereference reads far out of bounds. A ZIP central directory is allowed to contain zero-length name entries only if malformed, but producing such an archive is trivial. Both fixes reject the malformed entry and continue, so the parser skips bad archives rather than crashing.
1 parent 1d9ed2e commit 1235db3

1 file changed

Lines changed: 21 additions & 1 deletion

File tree

libretro-common/file/archive_file_zlib.c

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -359,7 +359,14 @@ static int zip_file_decompressed(
359359
uint32_t crc32, struct archive_extract_userdata *userdata)
360360
{
361361
decomp_state_t* decomp_state = (decomp_state_t*)userdata->cb_data;
362-
char last_char = name[strlen(name) - 1];
362+
size_t name_len = name ? strlen(name) : 0;
363+
char last_char;
364+
/* Reject empty or NULL name -- strlen-1 on empty would read
365+
* name[SIZE_MAX]. Malformed archives can have 0-length filename
366+
* entries. */
367+
if (name_len == 0)
368+
return 1;
369+
last_char = name[name_len - 1];
363370
/* Ignore directories. */
364371
if (last_char == '/' || last_char == '\\')
365372
return 1;
@@ -538,6 +545,12 @@ static int zip_parse_file_iterate_step_internal(
538545
if (entry < zip_context->directory || entry >= zip_context->directory_end)
539546
return 0;
540547

548+
/* Central-directory fixed header is 46 bytes (highest-offset read
549+
* is at +42..+45). Reject a truncated trailing entry before any
550+
* out-of-bounds read. */
551+
if ((size_t)(zip_context->directory_end - entry) < 46)
552+
return -1;
553+
541554
signature = read_le(zip_context->directory_entry + 0, 4);
542555

543556
if (signature != CENTRAL_FILE_HEADER_SIGNATURE)
@@ -555,6 +568,13 @@ static int zip_parse_file_iterate_step_internal(
555568
if (namelength >= PATH_MAX_LENGTH)
556569
return -1;
557570

571+
/* Variable-length fields (name, extra, comment) follow the 46-byte
572+
* fixed header. Reject if the declared sizes would run past the
573+
* end of the directory block. */
574+
if ((size_t)(zip_context->directory_end - entry)
575+
< (size_t)46 + namelength + extralength + commentlength)
576+
return -1;
577+
558578
memcpy(filename, zip_context->directory_entry + 46, namelength); /* file name */
559579
filename[namelength] = '\0';
560580

0 commit comments

Comments
 (0)