Skip to content

Commit

Permalink
Check for pure-ASCII UTF-8 strings that don’t match their normal coun…
Browse files Browse the repository at this point in the history 
…terpart.
  • Loading branch information
@dillof
dillof committed Aug 2, 2024
1 parent 343c15f commit d89c975
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 11 deletions.
35 changes: 29 additions & 6 deletions lib/zip_dirent.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
#include "zip.h"
#include "zipint.h"

static zip_string_t *_zip_dirent_process_ef_utf_8(const zip_dirent_t *de, zip_uint16_t id, zip_string_t *str);
static zip_string_t *_zip_dirent_process_ef_utf_8(const zip_dirent_t *de, zip_uint16_t id, zip_string_t *str, bool check_consistency);
static zip_extra_field_t *_zip_ef_utf8(zip_uint16_t, zip_string_t *, zip_error_t *);
static bool _zip_dirent_process_winzip_aes(zip_dirent_t *de, zip_error_t *error);

Expand Down Expand Up @@ -324,7 +324,7 @@ _zip_dirent_new(void) {
}


/* _zip_dirent_read(zde, fp, bufp, left, localp, error):
/*
Fills the zip directory entry zde.
If buffer is non-NULL, data is taken from there; otherwise data is read from fp as needed.
Expand All @@ -335,10 +335,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_uint64_t central_compressed_size, 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, bool check_consistency, zip_error_t *error) {
zip_uint8_t buf[CDENTRYSIZE];
zip_uint32_t size, variable_size;
zip_uint16_t filename_len, comment_len, ef_len;
zip_string_t *utf8_string;
bool is_zip64 = false;

bool from_buffer = (buffer != NULL);
Expand Down Expand Up @@ -506,8 +507,22 @@ _zip_dirent_read(zip_dirent_t *zde, zip_source_t *src, zip_buffer_t *buffer, boo
}
}

zde->filename = _zip_dirent_process_ef_utf_8(zde, ZIP_EF_UTF_8_NAME, zde->filename);
zde->comment = _zip_dirent_process_ef_utf_8(zde, ZIP_EF_UTF_8_COMMENT, zde->comment);
if ((utf8_string = _zip_dirent_process_ef_utf_8(zde, ZIP_EF_UTF_8_NAME, zde->filename, check_consistency)) == NULL && zde->filename != NULL) {
zip_error_set(error, ZIP_ER_INCONS, ZIP_ER_DETAIL_UTF8_FILENAME_MISMATCH);
if (!from_buffer) {
_zip_buffer_free(buffer);
}
return -1;
}
zde->filename = utf8_string;
if ((utf8_string = _zip_dirent_process_ef_utf_8(zde, ZIP_EF_UTF_8_COMMENT, zde->comment, check_consistency)) == NULL && zde->comment != NULL) {
zip_error_set(error, ZIP_ER_INCONS, ZIP_ER_DETAIL_UTF8_COMMENT_MISMATCH);
if (!from_buffer) {
_zip_buffer_free(buffer);
}
return -1;
}
zde->comment = utf8_string;

/* Zip64 */

Expand Down Expand Up @@ -644,7 +659,7 @@ zip_dirent_process_ef_zip64(zip_dirent_t *zde, const zip_uint8_t *ef, zip_uint64


static zip_string_t *
_zip_dirent_process_ef_utf_8(const zip_dirent_t *de, zip_uint16_t id, zip_string_t *str) {
_zip_dirent_process_ef_utf_8(const zip_dirent_t *de, zip_uint16_t id, zip_string_t *str, bool check_consistency) {
zip_uint16_t ef_len;
zip_uint32_t ef_crc;
zip_buffer_t *buffer;
Expand All @@ -667,6 +682,14 @@ _zip_dirent_process_ef_utf_8(const zip_dirent_t *de, zip_uint16_t id, zip_string
zip_string_t *ef_str = _zip_string_new(_zip_buffer_get(buffer, len), len, ZIP_FL_ENC_UTF_8, NULL);

if (ef_str != NULL) {
if (check_consistency) {
if (!_zip_string_equal(str, ef_str) && _zip_string_is_ascii(ef_str)) {
_zip_string_free(ef_str);
_zip_buffer_free(buffer);
return NULL;
}
}

_zip_string_free(str);
str = ef_str;
}
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 @@ -402,7 +402,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, 0, 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, za->open_flags & ZIP_CHECKCONS, 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 @@ -497,7 +497,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, cd->entry[i].orig->comp_size, error) == -1) {
if (_zip_dirent_read(&temp, za->src, NULL, true, cd->entry[i].orig->comp_size, true, 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
14 changes: 14 additions & 0 deletions lib/zip_string.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,20 @@ _zip_string_get(zip_string_t *string, zip_uint32_t *lenp, zip_flags_t flags, zip
return string->raw;
}

bool _zip_string_is_ascii(const zip_string_t *string) {
if (string->encoding != ZIP_ENCODING_ASCII) {
zip_uint16_t i;

for (i = 0; i < string->length; i++) {
if (string->raw[i] & 0x80) {
return false;
}
}
}

return true;
}


zip_uint16_t
_zip_string_length(const zip_string_t *s) {
Expand Down
5 changes: 4 additions & 1 deletion lib/zipint.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,8 @@ extern const int _zip_err_details_count;
#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 */
#define ZIP_ER_DETAIL_EOCD64_LOCATOR_MISMATCH 22 /* G EOCD64 and EOCD64 locator do not match */
#define ZIP_ER_DETAIL_UTF8_FILENAME_MISMATCH 23 /* E UTF-8 filename is ASCII and doesn't match filename */
#define ZIP_ER_DETAIL_UTF8_COMMENT_MISMATCH 24 /* E UTF-8 comment is ASCII and doesn't match comment */

/* directory entry: general purpose bit flags */

Expand Down Expand Up @@ -553,7 +555,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_uint64_t central_compressed_size, 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, bool check_consistency, 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 Expand Up @@ -639,6 +641,7 @@ int _zip_string_equal(const zip_string_t *a, const zip_string_t *b);
void _zip_string_free(zip_string_t *string);
zip_uint32_t _zip_string_crc32(const zip_string_t *string);
const zip_uint8_t *_zip_string_get(zip_string_t *string, zip_uint32_t *lenp, zip_flags_t flags, zip_error_t *error);
bool _zip_string_is_ascii(const zip_string_t *string);
zip_uint16_t _zip_string_length(const zip_string_t *string);
zip_string_t *_zip_string_new(const zip_uint8_t *raw, zip_uint16_t length, zip_flags_t flags, zip_error_t *error);
int _zip_string_write(zip_t *za, const zip_string_t *string);
Expand Down
6 changes: 4 additions & 2 deletions regress/open_incons.test
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ file incons-ef-local-dupe-zip64-v2.zzip incons-ef-local-dupe-zip64-v2.zip
file incons-ef-local-id-size.zzip incons-ef-local-id-size.zip
file incons-ef-local-id.zzip incons-ef-local-id.zip
file incons-ef-local-size.zzip incons-ef-local-size.zip
file incons-ef-local-utf8name-ascii.zzip incons-ef-local-utf8name-ascii.zip
file incons-eocd64.zzip incons-eocd64.zip
file incons-eocd-magic-bad.zzip incons-eocd-magic-bad.zip
file incons-file-count-high.zzip incons-file-count-high.zip
Expand All @@ -45,7 +46,7 @@ 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
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
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-ef-local-utf8name-ascii.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 @@ -73,6 +74,7 @@ opening 'incons-ef-local-dupe-zip64-v2.zzip' succeeded, 1 entries
opening 'incons-ef-local-id-size.zzip' returned error Zip archive inconsistent: entry 0: extra field length is invalid
opening 'incons-ef-local-id.zzip' succeeded, 1 entries
opening 'incons-ef-local-size.zzip' returned error Zip archive inconsistent: entry 0: extra field length is invalid
opening 'incons-ef-local-utf8name-ascii.zzip' returned error Zip archive inconsistent: entry 0: UTF-8 filename is ASCII and doesn't match filename
opening 'incons-eocd64.zzip' returned error Zip archive inconsistent: EOCD64 and EOCD do not match
opening 'incons-eocd-magic-bad.zzip' returned error Possibly truncated or corrupted zip archive
opening 'incons-file-count-high.zzip' returned error Zip archive inconsistent: central directory count of entries is incorrect
Expand All @@ -97,5 +99,5 @@ opening 'incons-streamed.zzip' returned error Zip archive inconsistent: entry 0:
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
37 errors
38 errors
end-of-inline-data

0 comments on commit d89c975

Please sign in to comment.