Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fail to read files at an offset greater than 4GB (infinite loop) #278

Open
nickbabcock opened this issue Jan 14, 2025 · 0 comments
Open
Labels
bug Something isn't working

Comments

@nickbabcock
Copy link
Contributor

nickbabcock commented Jan 14, 2025

Describe the bug
I have a large zip file (4GB+) that gets stuck in an infinite loop under ZipArchive::get_metadata

In lieu of a test case, here is a trace of the execution logic

When zip attempts to seek to a file:

zip2/src/read.rs

Lines 351 to 352 in 7c20fa3

// Go to start of data.
reader.seek(SeekFrom::Start(data.header_start))?;

It is seeking to a u64 derived from a u32:

zip2/src/read.rs

Line 1153 in 7c20fa3

header_start: offset.into(),

In this case header_start is 0xFFFF_FFFF. Which means the real offset is elsewhere:

   4.4.16 relative offset of local header: (4 bytes)

       This is the offset from the start of the first disk on
       which this file appears, to where the local header SHOULD
       be found.  If an archive is in ZIP64 format and the value
       in this field is 0xFFFFFFFF, the size will be in the 
       corresponding 8 byte zip64 extended information extra field.

Reading at this incorrect offset will cause the following to error due to an invalid signature

let block = ZipLocalEntryBlock::parse(reader)?;

This causes an infinite loop as the error is swallowed up and the code re-attempts the same exact read

zip2/src/read.rs

Lines 614 to 621 in 7c20fa3

// Turn EOCD into internal representation.
let Ok(shared) = CentralDirectoryInfo::try_from(&cde)
.and_then(|info| Self::read_central_header(info, config, reader))
else {
// The next EOCD candidate should start before the current one.
end_exclusive = cde.eocd.position;
continue;
};

This invalid behavior appears to be introduced in v2.1.4

Differs from #192 as zip-rs did not create the original zip file.

Desktop (please complete the following information):
Ubuntu 24.04

Additional context
Soft ping of folks who may be interested, no need to participate: @cosmicexplorer / @RisaI 😄

I'm still new to the zip file format, but I'm curious why zip-rs uses the central directory to determine the next entry's location when it is sequentially iterating through the entries anyways. Traversing the entries seems one after the other seems like a more foolproof strategy.

The alternative solution, I suppose, would be to parse out the offset from the zip64 extended information

@nickbabcock nickbabcock added the bug Something isn't working label Jan 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant