Skip to content

Commit

Permalink
Do not raise a zip bomb alert for a misplaced central directory.
Browse files Browse the repository at this point in the history
There is a zip-like file in the Firefox distribution, omni.ja,
which is a zip container with the central directory placed at the
start of the file instead of after the local entries as required
by the zip standard. This commit marks the actual location of the
central directory, as well as the end of central directory records,
as disallowed locations. This now permits such containers to not
raise a zip bomb alert, where in fact there are no overlaps.
  • Loading branch information
madler committed Jul 26, 2019
1 parent 6519bf0 commit 6d35183
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 6 deletions.
25 changes: 19 additions & 6 deletions extract.c
Original file line number Diff line number Diff line change
Expand Up @@ -493,8 +493,11 @@ int extract_or_test_files(__G) /* return PK-type error code */
}
#endif /* !SFX || SFX_EXDIR */

/* One more: initialize cover structure for bomb detection. Start with a
span that covers the central directory though the end of the file. */
/* One more: initialize cover structure for bomb detection. Start with
spans that cover any extra bytes at the start, the central directory,
the end of central directory record (including the Zip64 end of central
directory locator, if present), and the Zip64 end of central directory
record, if present. */
if (G.cover == NULL) {
G.cover = malloc(sizeof(cover_t));
if (G.cover == NULL) {
Expand All @@ -506,15 +509,25 @@ int extract_or_test_files(__G) /* return PK-type error code */
((cover_t *)G.cover)->max = 0;
}
((cover_t *)G.cover)->num = 0;
if ((G.extra_bytes != 0 &&
cover_add((cover_t *)G.cover, 0, G.extra_bytes) != 0) ||
cover_add((cover_t *)G.cover,
if (cover_add((cover_t *)G.cover,
G.extra_bytes + G.ecrec.offset_start_central_directory,
G.ziplen) != 0) {
G.extra_bytes + G.ecrec.offset_start_central_directory +
G.ecrec.size_central_directory) != 0) {
Info(slide, 0x401, ((char *)slide,
LoadFarString(NotEnoughMemCover)));
return PK_MEM;
}
if ((G.extra_bytes != 0 &&
cover_add((cover_t *)G.cover, 0, G.extra_bytes) != 0) ||
(G.ecrec.have_ecr64 &&
cover_add((cover_t *)G.cover, G.ecrec.ec64_start,
G.ecrec.ec64_end) != 0) ||
cover_add((cover_t *)G.cover, G.ecrec.ec_start,
G.ecrec.ec_end) != 0) {
Info(slide, 0x401, ((char *)slide,
LoadFarString(OverlappedComponents)));
return PK_BOMB;
}

/*---------------------------------------------------------------------------
The basic idea of this function is as follows. Since the central di-
Expand Down
6 changes: 6 additions & 0 deletions process.c
Original file line number Diff line number Diff line change
Expand Up @@ -1408,6 +1408,10 @@ static int find_ecrec64(__G__ searchlen) /* return PK-class error */

/* Now, we are (almost) sure that we have a Zip64 archive. */
G.ecrec.have_ecr64 = 1;
G.ecrec.ec_start -= ECLOC64_SIZE+4;
G.ecrec.ec64_start = ecrec64_start_offset;
G.ecrec.ec64_end = ecrec64_start_offset +
12 + makeint64(&byterec[ECREC64_LENGTH]);

/* Update the "end-of-central-dir offset" for later checks. */
G.real_ecrec_offset = ecrec64_start_offset;
Expand Down Expand Up @@ -1542,6 +1546,8 @@ static int find_ecrec(__G__ searchlen) /* return PK-class error */
makelong(&byterec[OFFSET_START_CENTRAL_DIRECTORY]);
G.ecrec.zipfile_comment_length =
makeword(&byterec[ZIPFILE_COMMENT_LENGTH]);
G.ecrec.ec_start = G.real_ecrec_offset;
G.ecrec.ec_end = G.ecrec.ec_start + 22 + G.ecrec.zipfile_comment_length;

/* Now, we have to read the archive comment, BEFORE the file pointer
is moved away backwards to seek for a Zip64 ECLOC64 structure.
Expand Down
10 changes: 10 additions & 0 deletions unzpriv.h
Original file line number Diff line number Diff line change
Expand Up @@ -2185,6 +2185,16 @@ typedef struct VMStimbuf {
int have_ecr64; /* valid Zip64 ecdir-record exists */
int is_zip64_archive; /* Zip64 ecdir-record is mandatory */
ush zipfile_comment_length;
zusz_t ec_start, ec_end; /* offsets of start and end of the
end of central directory record,
including if present the Zip64
end of central directory locator,
which immediately precedes the
end of central directory record */
zusz_t ec64_start, ec64_end; /* if have_ecr64 is true, then these
are the offsets of the start and
end of the Zip64 end of central
directory record */
} ecdir_rec;


Expand Down

0 comments on commit 6d35183

Please sign in to comment.