github-mirror/libarchive
1
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[Read core] Allow format readers to FAIL a header read

Browse Source 
While investigating improvements to Zip's header reading,
I found	an overlooked case in the read core:  When a format
returned FAILED	for a header read (indicating that the entry
could not be read but that the archive as a whole is still valid),
the core silently converted that to FATAL, ending the archive
at that	point.

The change to the read core adds a new internal state "DATA_RECOVERY"
indicating that we're post-header (like the existing DATA state) but
that client requests to actually obtain data are not valid.
This commit is contained in:
Tim Kientzle committed 2026-07-05 10:12:33 -07:00
1 parent 0409837cb9
commit 16d8156deb
3 files changed
+48 -3
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

libarchive/archive_check_magic.c
+1
View File
@@ -85,6 +85,7 @@ state_name(unsigned s)
case ARCHIVE_STATE_NEW: return ("new");
case ARCHIVE_STATE_HEADER: return ("header");
case ARCHIVE_STATE_DATA: return ("data");
case ARCHIVE_STATE_DATA_RECOVERY: return ("data_recovery");
case ARCHIVE_STATE_EOF: return ("eof");
case ARCHIVE_STATE_CLOSED: return ("closed");
case ARCHIVE_STATE_FATAL: return ("fatal");
libarchive/archive_private.h
+26
View File
@@ -61,12 +61,38 @@
#define ARCHIVE_READ_DISK_MAGIC (0xbadb0c5U)
#define ARCHIVE_MATCH_MAGIC (0xcad11c9U)
/*
* Having the state be a bitmask makes it easy to check
* for combinations of allowed states. These should
* generally only be used in public API entry points,
* primarily in archive_read.c, archive_write.c, etc.
* Internal callbacks can rely on the core machinery
* to only call them when appropriate.
*
* Generally checked via `__archive_check_magic()`.
*/
/* Newly created archive object, not yet opened */
#define ARCHIVE_STATE_NEW 1U
/* Archive is ready to read a header. */
#define ARCHIVE_STATE_HEADER 2U
/* A header has been read: client can ask for data
* or they can ask for the next header and
* we'll automatically skip the remaining data. */
#define ARCHIVE_STATE_DATA 4U
/* Similar to STATE_DATA but after a FAILED header:
* the client may not read data, but they may ask
* for the next header and we'll recover. */
#define ARCHIVE_STATE_DATA_RECOVERY 8U
/* End-of-archive has been reached. Client can only
* close or free the archive. */
#define ARCHIVE_STATE_EOF 0x10U
/* Archive is closed; client is only allowed to free the archive. */
#define ARCHIVE_STATE_CLOSED 0x20U
/* Archive is in a FATAL error state: a close request
* is permitted but ignored. */
#define ARCHIVE_STATE_FATAL 0x8000U
/* Any valid (non-fatal) state. */
#define ARCHIVE_STATE_ANY (0xFFFFU & ~ARCHIVE_STATE_FATAL)
struct archive_vtable {
libarchive/archive_read.c
+21 -3
View File
@@ -639,7 +639,8 @@ _archive_read_next_header2(struct archive *_a, struct archive_entry *entry)
int r1 = ARCHIVE_OK, r2;
archive_check_magic(_a, ARCHIVE_READ_MAGIC,
ARCHIVE_STATE_HEADER | ARCHIVE_STATE_DATA,
ARCHIVE_STATE_HEADER | ARCHIVE_STATE_DATA |
ARCHIVE_STATE_DATA_RECOVERY,
"archive_read_next_header");
archive_entry_clear(entry);
@@ -648,8 +649,12 @@ _archive_read_next_header2(struct archive *_a, struct archive_entry *entry)
/*
* If client didn't consume entire data, skip any remainder
* (This is especially important for GNU incremental directories.)
* A header that failed to parse (DATA_RECOVERY) still needs the
* same treatment: whatever of its body the format reader left
* unconsumed must be skipped before we can read the next header.
*/
if (a->archive.state == ARCHIVE_STATE_DATA) {
if (a->archive.state == ARCHIVE_STATE_DATA ||
a->archive.state == ARCHIVE_STATE_DATA_RECOVERY) {
r1 = archive_read_data_skip(&a->archive);
if (r1 == ARCHIVE_EOF)
archive_set_error(&a->archive, EIO,
@@ -687,6 +692,18 @@ _archive_read_next_header2(struct archive *_a, struct archive_entry *entry)
case ARCHIVE_FATAL:
a->archive.state = ARCHIVE_STATE_FATAL;
break;
case ARCHIVE_FAILED:
/*
* This entry's header could not be parsed, so its metadata
* cannot be trusted. ARCHIVE_STATE_DATA_RECOVERY still
* permits skipping past it (the format reader is
* responsible for ensuring that's actually possible), but
* blocks archive_read_data() and friends, which all check
* for ARCHIVE_STATE_DATA specifically and would otherwise
* return content for an entry we don't actually understand.
*/
a->archive.state = ARCHIVE_STATE_DATA_RECOVERY;
break;
}
if (r2 == ARCHIVE_OK || r2 == ARCHIVE_WARN)
@@ -953,7 +970,8 @@ archive_read_data_skip(struct archive *_a)
size_t size;
int64_t offset;
archive_check_magic(_a, ARCHIVE_READ_MAGIC, ARCHIVE_STATE_DATA,
archive_check_magic(_a, ARCHIVE_READ_MAGIC,
ARCHIVE_STATE_DATA | ARCHIVE_STATE_DATA_RECOVERY,
"archive_read_data_skip");
if (a->format->read_data_skip != NULL)