Fix invalid truncation error during extraction #21

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
2 participants
Contributor

bamiaux commented Sep 19, 2012

Only abort when bytes_avail is null and nothing remains into
zip->entry_bytes_remaining

@bamiaux bamiaux Fix invalid truncation error during extraction
Only abort when bytes_avail is null and nothing remains into
zip->entry_bytes_remaining
3797b8b
Contributor

kientzle commented Dec 13, 2013

There is something very strange here.

__archive_read_ahead() should never return a short read unless it has reached end-of-file.
What program is this part of?
Are you using a custom read callback?
Does your read callback ever return 0 bytes when it's not at end-of-file?
At the point where avail_bytes == 0, what is the buff pointer? (It should be NULL, which is an indication that __archive_read_ahead() was unable to satisfy the minimum request of 1 byte, which implies that it was at end-of-file.)

If there is a bug in __archive_read_ahead(), we need to identify that, as large parts of libarchive rely on the guarantee that it never returns short reads except at end-of-file.

Contributor

kientzle commented Dec 13, 2013

I'm also curious: Did your proposed fix actually change anything?

The zero value should be returned from this code in archive_read.c:

                    if (filter->end_of_file) {
                            if (avail != NULL)
                                    *avail = 0;
                            return (NULL);
                    }

Continuing to ask for more data after this point should have no effect.

Contributor

bamiaux commented May 28, 2014

Sorry for the delay. I don't have the file anymore and can't reproduce it.
Anyway:

  • It was used in a proprietary application, with custom callbacks
  • This callback was never meant to return 0 except at end-of-file. But I agree it could explain the bug. Except I vaguely remember testing this first.
  • Yes, the change did fix the bug, without, the zip was aborted with a truncated error.

However, this changeset looks wrong, the same condition exist in zip_read_data_deflate(), just before the call to inflate, and this is the one I've modified, not the one in zip_read_data_none. I must have messed the commit.

However, considering the delays, and that I can't reproduce it right now, feel free to close this issue.

Contributor

kientzle commented Jun 7, 2014

If you do find any more information, please let me know. If you (or anyone else) finds a file that demonstrates the problem (which you can share), please do pass it along.

kientzle closed this Jun 7, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment