Commit 654dd89d authored by David 'Digit' Turner's avatar David 'Digit' Turner Committed by Commit Bot

Fix generation of empty output jars on Unix.

When the input is a tiny empty .jar file (e.g. 22 bytes that
only include an empty End Of Central Directory record) as
shown below, ijar generated an invalid output, which was
supposed to only contain a single 'empty' file with length 0.

The problem was due to the fact that MappedOutputFile did
the following:

- First call ftruncate(fd, 22) to size the output file from
  an estimate of the input file (blindly assuming the result
  would be smaller).

- Later call ftruncate(fd, 108) to size the output with its
  'final' size. Unfortunately, the kernel interpreted that
  as _extending_ the file size, and overwriting the new
  bytes with zero.

In practice, this means that the buffer's content before
the second ftruncate() was:

	00000000: 50 4b 03 04 0a 00 00 00 00 00 00 00 00 00 00 00
	00000010: 00 00 00 00 00 00 00 00 00 00 05 00 00 00 64 75
	00000020: 6d 6d 79 50 4b 01 02 00 00 0a 00 00 00 00 00 00
	00000030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 05
	00000040: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
	00000050: 00 64 75 6d 6d 79 50 4b 05 06 00 00 00 00 01 00
	00000060: 01 00 33 00 00 00 23 00 00 00 00 00

And after ftruncate() became:

	00000000: 50 4b 03 04 0a 00 00 00 00 00 00 00 00 00 00 00
	00000010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
	00000020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
	00000030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
	00000040: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
	00000050: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
	00000060: 00 00 00 00 00 00 00 00 00 00 00 00

The result only contains a Local File Header record, but has no
longer a final End Of Central Directory record, and is thus no
longer a valid zip archive!

This is fixed by this CL by ensuring that the initial ftruncate()
is at least one page long. A sanity check is also added to return
an error if the output file is larger than its input for some
unexpected reason.

This is not needed on Windows, which closes the file then
later changes its size.

-- example tiny input zip file of 22
00000000: 504b 0506 0000 0000 0000 0000 0000 0000  PK..............
00000010: 0000 0000 0000

BUG=925257
R=agrieve@chromium.org,dpranke@chromium.org,benmason@chromium.org

Change-Id: If06b3ca88fee2dceaebacb427741ef48311cb38d
Reviewed-on: https://chromium-review.googlesource.com/c/1436361Reviewed-by: default avatarAndrew Grieve <agrieve@chromium.org>
Commit-Queue: Andrew Grieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#626100}
parent 6fa75b56
...@@ -14,3 +14,4 @@ Local Modifications: ...@@ -14,3 +14,4 @@ Local Modifications:
- Added BUILD.gn - Added BUILD.gn
- Enabled CRC32 zip entries by passing true to 3rd parameter of FinishFile() - Enabled CRC32 zip entries by passing true to 3rd parameter of FinishFile()
within ijar.cc within ijar.cc
- Avoid generating invalid empty zip archive (http://crbug.com/925257)
...@@ -87,6 +87,7 @@ int MappedInputFile::Close() { ...@@ -87,6 +87,7 @@ int MappedInputFile::Close() {
struct MappedOutputFileImpl { struct MappedOutputFileImpl {
int fd_; int fd_;
u8 initial_length_;
}; };
MappedOutputFile::MappedOutputFile(const char* name, u8 estimated_size) { MappedOutputFile::MappedOutputFile(const char* name, u8 estimated_size) {
...@@ -99,6 +100,14 @@ MappedOutputFile::MappedOutputFile(const char* name, u8 estimated_size) { ...@@ -99,6 +100,14 @@ MappedOutputFile::MappedOutputFile(const char* name, u8 estimated_size) {
return; return;
} }
const size_t page_size = sysconf(_SC_PAGESIZE);
// Ensure estimated_size is always at least one page to avoid issues
// when a tiny empty input file is larger than its output.
// http://crbug.com/925257
if (estimated_size < page_size)
estimated_size = page_size;
// Create mmap-able sparse file // Create mmap-able sparse file
if (ftruncate(fd, estimated_size) < 0) { if (ftruncate(fd, estimated_size) < 0) {
snprintf(errmsg, MAX_ERROR, "ftruncate(): %s", strerror(errno)); snprintf(errmsg, MAX_ERROR, "ftruncate(): %s", strerror(errno));
...@@ -108,8 +117,8 @@ MappedOutputFile::MappedOutputFile(const char* name, u8 estimated_size) { ...@@ -108,8 +117,8 @@ MappedOutputFile::MappedOutputFile(const char* name, u8 estimated_size) {
// Ensure that any buffer overflow in JarStripper will result in // Ensure that any buffer overflow in JarStripper will result in
// SIGSEGV or SIGBUS by over-allocating beyond the end of the file. // SIGSEGV or SIGBUS by over-allocating beyond the end of the file.
size_t mmap_length = std::min(estimated_size + sysconf(_SC_PAGESIZE), size_t mmap_length = std::min(estimated_size + page_size,
(u8) std::numeric_limits<size_t>::max()); (u8)std::numeric_limits<size_t>::max());
void* mapped = mmap(NULL, mmap_length, PROT_WRITE, MAP_SHARED, fd, 0); void* mapped = mmap(NULL, mmap_length, PROT_WRITE, MAP_SHARED, fd, 0);
if (mapped == MAP_FAILED) { if (mapped == MAP_FAILED) {
snprintf(errmsg, MAX_ERROR, "mmap(): %s", strerror(errno)); snprintf(errmsg, MAX_ERROR, "mmap(): %s", strerror(errno));
...@@ -119,6 +128,7 @@ MappedOutputFile::MappedOutputFile(const char* name, u8 estimated_size) { ...@@ -119,6 +128,7 @@ MappedOutputFile::MappedOutputFile(const char* name, u8 estimated_size) {
impl_ = new MappedOutputFileImpl(); impl_ = new MappedOutputFileImpl();
impl_->fd_ = fd; impl_->fd_ = fd;
impl_->initial_length_ = estimated_size;
buffer_ = reinterpret_cast<u1*>(mapped); buffer_ = reinterpret_cast<u1*>(mapped);
opened_ = true; opened_ = true;
} }
...@@ -129,6 +139,14 @@ MappedOutputFile::~MappedOutputFile() { ...@@ -129,6 +139,14 @@ MappedOutputFile::~MappedOutputFile() {
} }
int MappedOutputFile::Close(int size) { int MappedOutputFile::Close(int size) {
if (static_cast<u8>(size) > impl_->initial_length_) {
snprintf(errmsg, MAX_ERROR,
"Output file is larger than its input (%d > %lld)\n", size,
static_cast<long long>(impl_->initial_length_));
errmsg_ = errmsg;
return -1;
}
if (ftruncate(impl_->fd_, size) < 0) { if (ftruncate(impl_->fd_, size) < 0) {
snprintf(errmsg, MAX_ERROR, "ftruncate(): %s", strerror(errno)); snprintf(errmsg, MAX_ERROR, "ftruncate(): %s", strerror(errno));
errmsg_ = errmsg; errmsg_ = errmsg;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment