Skip to content

Commit

Permalink
Check for consistency of data descriptor.
Browse files Browse the repository at this point in the history
  • Loading branch information
@dillof
dillof committed Aug 2, 2024
1 parent 466aa15 commit 39517cc
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 24 deletions.
70 changes: 52 additions & 18 deletions lib/zip_dirent.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -340,10 +340,11 @@ _zip_dirent_new(void) {
*/

zip_int64_t
_zip_dirent_read(zip_dirent_t *zde, zip_source_t *src, zip_buffer_t *buffer, bool local, zip_error_t *error) {
_zip_dirent_read(zip_dirent_t *zde, zip_source_t *src, zip_buffer_t *buffer, bool local, zip_uint64_t central_compressed_size, zip_error_t *error) {
zip_uint8_t buf[CDENTRYSIZE];
zip_uint32_t size, variable_size;
zip_uint16_t filename_len, comment_len, ef_len;
bool is_zip64 = false;

bool from_buffer = (buffer != NULL);

Expand Down Expand Up @@ -526,6 +527,7 @@ _zip_dirent_read(zip_dirent_t *zde, zip_source_t *src, zip_buffer_t *buffer, boo
return -1;
}
}
is_zip64 = true;
}


Expand All @@ -536,10 +538,40 @@ _zip_dirent_read(zip_dirent_t *zde, zip_source_t *src, zip_buffer_t *buffer, boo
}
return -1;
}

if (!from_buffer) {
_zip_buffer_free(buffer);
}

if (local && zde->bitflags & ZIP_GPBF_DATA_DESCRIPTOR) {
zip_uint32_t df_crc;
zip_uint64_t df_comp_size, df_uncomp_size;
if (zip_source_seek(src, central_compressed_size, SEEK_CUR) != 0 || (buffer = _zip_buffer_new_from_source(src, MAX_DATA_DESCRIPTOR_LENGTH, buf, error)) == NULL) {
return -1;
}
if (memcmp(_zip_buffer_peek(buffer, MAGIC_LEN), DATADES_MAGIC, MAGIC_LEN) == 0) {
_zip_buffer_skip(buffer, MAGIC_LEN);
}
df_crc = _zip_buffer_get_32(buffer);
df_comp_size = is_zip64 ? _zip_buffer_get_64(buffer) : _zip_buffer_get_32(buffer);
df_uncomp_size = is_zip64 ? _zip_buffer_get_64(buffer) : _zip_buffer_get_32(buffer);

if (!_zip_buffer_ok(buffer)) {
zip_error_set(error, ZIP_ER_INTERNAL, 0);
_zip_buffer_free(buffer);
return -1;
}
_zip_buffer_free(buffer);

if ((zde->crc != 0 && zde->crc != df_crc) || (zde->comp_size != 0 && zde->comp_size != df_comp_size) || (zde->uncomp_size != 0 && zde->uncomp_size != df_uncomp_size)) {
zip_error_set(error, ZIP_ER_INCONS, ZIP_ER_DETAIL_DATA_DESCRIPTOR_MISMATCH);
return -1;
}
zde->crc = df_crc;
zde->comp_size = df_comp_size;
zde->uncomp_size = df_uncomp_size;
}

/* zip_source_seek / zip_source_tell don't support values > ZIP_INT64_MAX */
if (zde->offset > ZIP_INT64_MAX) {
zip_error_set(error, ZIP_ER_SEEK, EFBIG);
Expand All @@ -555,7 +587,8 @@ _zip_dirent_read(zip_dirent_t *zde, zip_source_t *src, zip_buffer_t *buffer, boo
return (zip_int64_t)size + (zip_int64_t)variable_size;
}

bool zip_dirent_process_ef_zip64(zip_dirent_t* zde, const zip_uint8_t* ef, zip_uint64_t got_len, bool local, zip_error_t* error) {
bool
zip_dirent_process_ef_zip64(zip_dirent_t *zde, const zip_uint8_t *ef, zip_uint64_t got_len, bool local, zip_error_t *error) {
zip_buffer_t *ef_buffer;

if ((ef_buffer = _zip_buffer_new((zip_uint8_t *)ef, got_len)) == NULL) {
Expand Down Expand Up @@ -679,18 +712,18 @@ _zip_dirent_process_winzip_aes(zip_dirent_t *de, zip_error_t *error) {

crc_valid = true;
switch (_zip_buffer_get_16(buffer)) {
case 1:
break;
case 1:
break;

case 2:
crc_valid = false;
/* TODO: When checking consistency, check that crc is 0. */
break;
default:
zip_error_set(error, ZIP_ER_ENCRNOTSUPP, 0);
_zip_buffer_free(buffer);
return false;
case 2:
crc_valid = false;
/* TODO: When checking consistency, check that crc is 0. */
break;

default:
zip_error_set(error, ZIP_ER_ENCRNOTSUPP, 0);
_zip_buffer_free(buffer);
return false;
}

/* vendor */
Expand Down Expand Up @@ -1025,7 +1058,7 @@ _zip_dirent_write(zip_t *za, zip_dirent_t *de, zip_flags_t flags) {


time_t
_zip_d2u_time(const zip_dostime_t* dtime) {
_zip_d2u_time(const zip_dostime_t *dtime) {
struct tm tm;

memset(&tm, 0, sizeof(tm));
Expand Down Expand Up @@ -1188,21 +1221,22 @@ _zip_dirent_apply_attributes(zip_dirent_t *de, zip_file_attributes_t *attributes
Set values suitable for torrentzip.
*/

void zip_dirent_torrentzip_normalize(zip_dirent_t *de) {
void
zip_dirent_torrentzip_normalize(zip_dirent_t *de) {
de->version_madeby = 0;
de->version_needed = 20; /* 2.0 */
de->bitflags = 2; /* maximum compression */
de->bitflags = 2; /* maximum compression */
de->comp_method = ZIP_CM_DEFLATE;
de->compression_level = TORRENTZIP_COMPRESSION_FLAGS;
de->disk_number = 0;
de->int_attrib = 0;
de->ext_attrib = 0;

/* last_mod, extra_fields, and comment are normalized in zip_dirent_write() directly */

}

int zip_dirent_check_consistency(zip_dirent_t *dirent) {
int
zip_dirent_check_consistency(zip_dirent_t *dirent) {
if (dirent->comp_method == ZIP_CM_STORE && dirent->comp_size != dirent->uncomp_size) {
return ZIP_ER_DETAIL_STORED_SIZE_MISMATCH;
}
Expand Down
4 changes: 2 additions & 2 deletions lib/zip_open.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,7 @@ _zip_read_cdir(zip_t *za, zip_buffer_t *buffer, zip_uint64_t buf_offset, zip_err
grown = true;
}

if ((cd->entry[i].orig = _zip_dirent_new()) == NULL || (entry_size = _zip_dirent_read(cd->entry[i].orig, za->src, cd_buffer, false, error)) < 0) {
if ((cd->entry[i].orig = _zip_dirent_new()) == NULL || (entry_size = _zip_dirent_read(cd->entry[i].orig, za->src, cd_buffer, false, 0, error)) < 0) {
if (zip_error_code_zip(error) == ZIP_ER_INCONS) {
zip_error_set(error, ZIP_ER_INCONS, ADD_INDEX_TO_DETAIL(zip_error_code_system(error), i));
}
Expand Down Expand Up @@ -487,7 +487,7 @@ _zip_checkcons(zip_t *za, zip_cdir_t *cd, zip_error_t *error) {
return -1;
}

if (_zip_dirent_read(&temp, za->src, NULL, true, error) == -1) {
if (_zip_dirent_read(&temp, za->src, NULL, true, cd->entry[i].orig->comp_size, error) == -1) {
if (zip_error_code_zip(error) == ZIP_ER_INCONS) {
zip_error_set(error, ZIP_ER_INCONS, ADD_INDEX_TO_DETAIL(zip_error_code_system(error), i));
}
Expand Down
3 changes: 2 additions & 1 deletion lib/zipint.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ extern const int _zip_err_details_count;
#define ZIP_ER_DETAIL_INVALID_EF_LENGTH 18 /* E extra field length is invalid */
#define ZIP_ER_DETAIL_INVALID_FILE_LENGTH 19 /* E file length in header doesn't match actual file length */
#define ZIP_ER_DETAIL_STORED_SIZE_MISMATCH 20 /* E compressed and uncompressed sizes don't match for stored file */
#define ZIP_ER_DETAIL_DATA_DESCRIPTOR_MISMATCH 21 /* E local header and data descriptor do not match */

/* directory entry: general purpose bit flags */

Expand Down Expand Up @@ -548,7 +549,7 @@ void _zip_dirent_init(zip_dirent_t *);
bool _zip_dirent_needs_zip64(const zip_dirent_t *, zip_flags_t);
zip_dirent_t *_zip_dirent_new(void);
bool zip_dirent_process_ef_zip64(zip_dirent_t * zde, const zip_uint8_t * ef, zip_uint64_t got_len, bool local, zip_error_t * error);
zip_int64_t _zip_dirent_read(zip_dirent_t *zde, zip_source_t *src, zip_buffer_t *buffer, bool local, zip_error_t *error);
zip_int64_t _zip_dirent_read(zip_dirent_t *zde, zip_source_t *src, zip_buffer_t *buffer, bool local, zip_uint64_t central_compressed_size, zip_error_t *error);
void _zip_dirent_set_version_needed(zip_dirent_t *de, bool force_zip64);
void zip_dirent_torrentzip_normalize(zip_dirent_t *de);

Expand Down
8 changes: 5 additions & 3 deletions regress/open_incons.test
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ file incons-local-magic-bad.zzip incons-local-magic-bad.zip
file incons-local-size-larger.zzip incons-local-size-larger.zip
file incons-stored-size.zzip incons-stored-size.zip
file incons-streamed.zzip incons-streamed.zip
arguments -s -c incons-archive-comment-longer.zzip incons-archive-comment-shorter.zzip incons-cdoffset.zzip incons-central-compression-method.zzip incons-central-compsize-larger-toolarge.zzip incons-central-compsize-larger.zzip incons-central-compsize-smaller.zzip incons-central-crc.zzip incons-central-date.zzip incons-central-file-comment-longer.zzip incons-central-file-comment-shorter.zzip incons-central-magic-bad.zzip incons-central-magic-bad2.zzip incons-central-size-larger.zzip incons-data.zzip incons-ef-central-size-wrong.zzip incons-ef-local-dupe-utf8comment.zzip incons-ef-local-dupe-utf8name.zzip incons-ef-local-dupe-zip64-v1.zzip incons-ef-local-dupe-zip64-v2.zzip incons-ef-local-id-size.zzip incons-ef-local-id.zzip incons-ef-local-size.zzip incons-eocd64.zzip incons-eocd-magic-bad.zzip incons-file-count-high.zzip incons-file-count-low.zzip incons-file-count-overflow.zzip incons-gap-before-cd.zzip incons-gap-before-eocd.zzip incons-gap-before-local.zzip incons-local-compression-method.zzip incons-local-compsize-larger.zzip incons-local-compsize-smaller.zzip incons-local-crc.zzip incons-local-filename-long.zzip incons-local-filename-missing.zzip incons-local-filename-nil-byte.zzip incons-local-filename-short.zzip incons-local-filename.zzip incons-local-magic-bad.zzip incons-local-size-larger.zzip incons-stored-size.zzip incons-streamed.zzip
file incons-streamed-2.zzip incons-streamed-2.zip
arguments -s -c incons-archive-comment-longer.zzip incons-archive-comment-shorter.zzip incons-cdoffset.zzip incons-central-compression-method.zzip incons-central-compsize-larger-toolarge.zzip incons-central-compsize-larger.zzip incons-central-compsize-smaller.zzip incons-central-crc.zzip incons-central-date.zzip incons-central-file-comment-longer.zzip incons-central-file-comment-shorter.zzip incons-central-magic-bad.zzip incons-central-magic-bad2.zzip incons-central-size-larger.zzip incons-data.zzip incons-ef-central-size-wrong.zzip incons-ef-local-dupe-utf8comment.zzip incons-ef-local-dupe-utf8name.zzip incons-ef-local-dupe-zip64-v1.zzip incons-ef-local-dupe-zip64-v2.zzip incons-ef-local-id-size.zzip incons-ef-local-id.zzip incons-ef-local-size.zzip incons-eocd64.zzip incons-eocd-magic-bad.zzip incons-file-count-high.zzip incons-file-count-low.zzip incons-file-count-overflow.zzip incons-gap-before-cd.zzip incons-gap-before-eocd.zzip incons-gap-before-local.zzip incons-local-compression-method.zzip incons-local-compsize-larger.zzip incons-local-compsize-smaller.zzip incons-local-crc.zzip incons-local-filename-long.zzip incons-local-filename-missing.zzip incons-local-filename-nil-byte.zzip incons-local-filename-short.zzip incons-local-filename.zzip incons-local-magic-bad.zzip incons-local-size-larger.zzip incons-stored-size.zzip incons-streamed.zzip incons-streamed-2.zzip
return 1
# tryopen does not test checksums, so this is fine.
# different extra fields local vs. central is fine
Expand Down Expand Up @@ -92,8 +93,9 @@ opening 'incons-local-filename.zzip' returned error Zip archive inconsistent: en
opening 'incons-local-magic-bad.zzip' returned error Not a zip archive
opening 'incons-local-size-larger.zzip' returned error Zip archive inconsistent: entry 0: local and central headers do not match
opening 'incons-stored-size.zzip' returned error Zip archive inconsistent: entry 0: compressed and uncompressed sizes don't match for stored file
opening 'incons-streamed.zzip' succeeded, 1 entries
opening 'incons-streamed.zzip' returned error Zip archive inconsistent: entry 0: local and central headers do not match
opening 'incons-streamed-2.zzip' returned error Zip archive inconsistent: entry 0: local header and data descriptor do not match
end-of-inline-data
stderr
34 errors
36 errors
end-of-inline-data

0 comments on commit 39517cc

Please sign in to comment.