From 0ff100c8d32632d2564615114207a474cea8abe5 Mon Sep 17 00:00:00 2001 From: Grzegorz Antoniak Date: Fri, 1 Aug 2025 22:02:30 +0200 Subject: [PATCH] RAR5 reader: fix multiple issues in extra field parsing function This commit fixes multiple issues found in the function that parses extra fields found in the "file"/"service" blocks. 1. In case the file declares just one extra field, which is an unsupported field, the function returns ARCHIVE_FATAL. The commit fixes this so this case is allowed, and the unsupported extra field is skipped. The commit also introduces a test for this case. 2. Current parsing method of extra fields can report parsing errors in case the file is malformed. The problem is that next iteration of parsing, which is meant to process the next extra field (if any), overwrites the result of the previous iteration, even if previous iteration has reported parsing error. A successful parse can be returned in this case, leading to undefined behavior. This commit changes the behavior to fail the parsing function early. Also a test file is introduced for this case. 3. In case the file declares only the EX_CRYPT extra field, current function simply returns ARCHIVE_FATAL, preventing the caller from setting the proper error string. This results in libarchive returning an ARCHIVE_FATAL without any error messages set. The PR #2096 (commit adee36b00) was specifically created to provide error strings in case EX_CRYPT attribute was encountered, but current behavior contradicts this case. The commit changes the behavior so that ARCHIVE_OK is returned by the extra field parsing function in only EX_CRYPT is encountered, so that the caller header reading function can properly return ARCHIVE_FATAL to the caller, at the same time setting a proper error string. A test file is also provided for this case. This PR should fix issue #2711. --- Makefile.am | 3 ++ libarchive/archive_read_support_format_rar5.c | 25 +++++++-- libarchive/test/test_read_format_rar5.c | 54 +++++++++++++++++++ ...rar5_invalid_hash_valid_htime_exfld.rar.uu | 6 +++ ...t_read_format_rar5_only_crypt_exfld.rar.uu | 7 +++ ..._read_format_rar5_unsupported_exfld.rar.uu | 6 +++ 6 files changed, 97 insertions(+), 4 deletions(-) create mode 100644 libarchive/test/test_read_format_rar5_invalid_hash_valid_htime_exfld.rar.uu create mode 100644 libarchive/test/test_read_format_rar5_only_crypt_exfld.rar.uu create mode 100644 libarchive/test/test_read_format_rar5_unsupported_exfld.rar.uu diff --git a/Makefile.am b/Makefile.am index 9dd37bb30..9d8873d1d 100644 --- a/Makefile.am +++ b/Makefile.am @@ -921,6 +921,9 @@ libarchive_test_EXTRA_DIST=\ libarchive/test/test_read_format_rar4_encrypted_filenames.rar.uu \ libarchive/test/test_read_format_rar4_solid_encrypted.rar.uu \ libarchive/test/test_read_format_rar4_solid_encrypted_filenames.rar.uu \ + libarchive/test/test_read_format_rar5_only_crypt_exfld.rar.uu \ + libarchive/test/test_read_format_rar5_unsupported_exfld.rar.uu \ + libarchive/test/test_read_format_rar5_invalid_hash_valid_htime_exfld.rar.uu \ libarchive/test/test_read_format_rar5_encrypted.rar.uu \ libarchive/test/test_read_format_rar5_encrypted_filenames.rar.uu \ libarchive/test/test_read_format_rar5_solid_encrypted.rar.uu \ diff --git a/libarchive/archive_read_support_format_rar5.c b/libarchive/archive_read_support_format_rar5.c index 48dde0c2e..f2af82cfa 100644 --- a/libarchive/archive_read_support_format_rar5.c +++ b/libarchive/archive_read_support_format_rar5.c @@ -1619,10 +1619,13 @@ static int process_head_file_extra(struct archive_read* a, { uint64_t extra_field_size; uint64_t extra_field_id = 0; - int ret = ARCHIVE_FATAL; uint64_t var_size; while(extra_data_size > 0) { + /* Make sure we won't fail if the file declares only unsupported + attributes. */ + int ret = ARCHIVE_OK; + if(!read_var(a, &extra_field_size, &var_size)) return ARCHIVE_EOF; @@ -1675,12 +1678,26 @@ static int process_head_file_extra(struct archive_read* a, if (ARCHIVE_OK != consume(a, extra_field_size)) { return ARCHIVE_EOF; } + + /* Don't fail on unsupported attribute -- we've handled it + by skipping over it. */ + ret = ARCHIVE_OK; + } + + if (ret != ARCHIVE_OK) { + /* Forward any errors signalled by the attribute parsing + functions. */ + return ret; } } - if(ret != ARCHIVE_OK) { - /* Attribute not implemented. */ - return ret; + if (extra_data_size != 0) { + /* We didn't skip everything, or we skipped too much; either way, + there's an error in this parsing function. */ + + archive_set_error(&a->archive, ARCHIVE_ERRNO_PROGRAMMER, + "unsupported structure of file header extra data"); + return ARCHIVE_FATAL; } return ARCHIVE_OK; diff --git a/libarchive/test/test_read_format_rar5.c b/libarchive/test/test_read_format_rar5.c index fd233277b..202b66f68 100644 --- a/libarchive/test/test_read_format_rar5.c +++ b/libarchive/test/test_read_format_rar5.c @@ -1428,3 +1428,57 @@ DEFINE_TEST(test_read_format_rar5_data_ready_pointer_leak) EPILOGUE(); } + +DEFINE_TEST(test_read_format_rar5_only_crypt_exfld) +{ + /* GH #2711 */ + + char buf[4096]; + PROLOGUE("test_read_format_rar5_only_crypt_exfld.rar"); + + /* The reader should allow iteration through files, but should fail + during data extraction. */ + + assertA(archive_read_next_header(a, &ae) == ARCHIVE_OK); + assertA(archive_read_data(a, buf, sizeof(buf)) == ARCHIVE_FATAL); + + /* The reader should also provide a valid error message. */ + assertA(archive_error_string(a) != NULL); + + EPILOGUE(); +} + +DEFINE_TEST(test_read_format_rar5_only_unsupported_exfld) +{ + /* GH #2711 */ + + char buf[4096]; + PROLOGUE("test_read_format_rar5_unsupported_exfld.rar"); + + /* The reader should allow iteration through files, and it should + succeed with data extraction. */ + + assertA(archive_read_next_header(a, &ae) == ARCHIVE_OK); + + /* 48 is the expected number of bytes that should be extracted */ + assertA(archive_read_data(a, buf, sizeof(buf)) == 48); + + EPILOGUE(); +} + +DEFINE_TEST(test_read_format_rar5_invalidhash_and_validhtime_exfld) +{ + /* GH #2711 */ + + char buf[4096]; + PROLOGUE("test_read_format_rar5_invalid_hash_valid_htime_exfld.rar"); + + /* The reader should report an error when trying to process this data. + Returning EOF here means that the reader has failed to identify + malformed structure. */ + + assertA(archive_read_next_header(a, &ae) < 0); + assertA(archive_read_data(a, buf, sizeof(buf)) < 0); + + EPILOGUE(); +} diff --git a/libarchive/test/test_read_format_rar5_invalid_hash_valid_htime_exfld.rar.uu b/libarchive/test/test_read_format_rar5_invalid_hash_valid_htime_exfld.rar.uu new file mode 100644 index 000000000..399acd814 --- /dev/null +++ b/libarchive/test/test_read_format_rar5_invalid_hash_valid_htime_exfld.rar.uu @@ -0,0 +1,6 @@ +begin 644 - +M4F%R(1H'`0`SDK7E"@$%!@`%`0&`@`#^T/5L)`(###$$,>V#`D840I@``0AF +M:6QE+G1X=`@"OX0]``$"`P(#`&EN=F%L:60@2$%32"!E>'1R82P@86YD(&QA +>=&5R(&$@=F%L:60@2%1)344@97AT)!,``0AF +M:6QE+G1X="0!```&``````````````````````````````````````````!R +M87(U('-T;W)E9"!F:6QE('=I=&@@;VYL>2!A($-265!4(&5X=')A(&9I96QD +'(#W?A@(%!``` +` +end diff --git a/libarchive/test/test_read_format_rar5_unsupported_exfld.rar.uu b/libarchive/test/test_read_format_rar5_unsupported_exfld.rar.uu new file mode 100644 index 000000000..16b456bf4 --- /dev/null +++ b/libarchive/test/test_read_format_rar5_unsupported_exfld.rar.uu @@ -0,0 +1,6 @@ +begin 644 - +M4F%R(1H'`0`SDK7E"@$%!@`%`0&`@`#>[JDS)@(##C`$,.V#`BX6Z[0``0AF +M:6QE+G1X=`W_____#WA6-!(`````