drgreen/android_frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

AAPT2: Ignore trailing data after IEND chunk in PNG

Browse Source 
libpng seems to do the same, so our chunk filter should not care what
comes after.

Bug: 38169876
Test: manual
Change-Id: I020ddfe29ac3eb4d5ef9ace5611c6f6497de443d
This commit is contained in:
Adam Lesinski
2017-05-12 18:16:44 -07:00
parent 33532e0208
commit cc73e990e5
4 changed files with 59 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
tools/aapt2/cmd/Compile.cpp
View File

@ -372,6 +372,8 @@ static bool IsValidFile(IAaptContext* context, const StringPiece& input_path) {
if (file_type == file::FileType::kDirectory) {
context->GetDiagnostics()->Error(DiagMessage(input_path)
<< "resource file cannot be a directory");
} else if (file_type == file::FileType::kNonexistant) {
context->GetDiagnostics()->Error(DiagMessage(input_path) << "file not found");
} else {
context->GetDiagnostics()->Error(DiagMessage(input_path)
<< "not a valid resource file");
@ -488,7 +490,7 @@ static bool CompilePng(IAaptContext* context, const CompileOptions& options,
// Ensure that we only keep the chunks we care about if we end up
// using the original PNG instead of the crunched one.
PngChunkFilter png_chunk_filter(content);
std::unique_ptr<Image> image = ReadPng(context, &png_chunk_filter);
std::unique_ptr<Image> image = ReadPng(context, path_data.source, &png_chunk_filter);
if (!image) {
return false;
}

11
tools/aapt2/compile/Png.h
View File

@ -69,7 +69,12 @@ class PngChunkFilter : public io::InputStream {
bool Rewind() override;
size_t ByteCount() const override { return window_start_; }
bool HadError() const override { return error_; }
bool HadError() const override {
return !error_msg_.empty();
}
std::string GetError() const override {
return error_msg_;
}
private:
DISALLOW_COPY_AND_ASSIGN(PngChunkFilter);
@ -79,13 +84,13 @@ class PngChunkFilter : public io::InputStream {
android::StringPiece data_;
size_t window_start_ = 0;
size_t window_end_ = 0;
bool error_ = false;
std::string error_msg_;
};
/**
* Reads a PNG from the InputStream into memory as an RGBA Image.
*/
std::unique_ptr<Image> ReadPng(IAaptContext* context, io::InputStream* in);
std::unique_ptr<Image> ReadPng(IAaptContext* context, const Source& source, io::InputStream* in);
/**
* Writes the RGBA Image, with optional 9-patch meta-data, into the OutputStream

33
tools/aapt2/compile/PngChunkFilter.cpp
View File

@ -16,11 +16,13 @@
#include "compile/Png.h"
#include "android-base/stringprintf.h"
#include "androidfw/StringPiece.h"
#include "io/Io.h"
using android::StringPiece;
using android::base::StringPrintf;
namespace aapt {
@ -73,7 +75,7 @@ PngChunkFilter::PngChunkFilter(const StringPiece& data) : data_(data) {
window_start_ = 0;
window_end_ = kPngSignatureSize;
} else {
error_ = true;
error_msg_ = "PNG does not start with PNG signature";
}
}
@ -90,7 +92,7 @@ bool PngChunkFilter::ConsumeWindow(const void** buffer, size_t* len) {
}
bool PngChunkFilter::Next(const void** buffer, size_t* len) {
if (error_) {
if (HadError()) {
return false;
}
@ -106,16 +108,21 @@ bool PngChunkFilter::Next(const void** buffer, size_t* len) {
const size_t kMinChunkHeaderSize = 3 * sizeof(uint32_t);
// Is there enough room for a chunk header?
if (data_.size() - window_start_ < kMinChunkHeaderSize) {
error_ = true;
if (data_.size() - window_end_ < kMinChunkHeaderSize) {
error_msg_ = StringPrintf("Not enough space for a PNG chunk @ byte %zu/%zu", window_end_,
data_.size());
return false;
}
// Verify the chunk length.
const uint32_t chunk_len = Peek32LE(data_.data() + window_end_);
if (((uint64_t)chunk_len) + ((uint64_t)window_end_) + sizeof(uint32_t) > data_.size()) {
if ((size_t)chunk_len > data_.size() - window_end_ - kMinChunkHeaderSize) {
// Overflow.
error_ = true;
const uint32_t chunk_type = Peek32LE(data_.data() + window_end_ + sizeof(uint32_t));
error_msg_ = StringPrintf(
"PNG chunk type %08x is too large: chunk length is %zu but chunk "
"starts at byte %zu/%zu",
chunk_type, (size_t)chunk_len, window_end_ + kMinChunkHeaderSize, data_.size());
return false;
}
@ -124,6 +131,16 @@ bool PngChunkFilter::Next(const void** buffer, size_t* len) {
if (IsPngChunkWhitelisted(chunk_type)) {
// Advance the window to include this chunk.
window_end_ += kMinChunkHeaderSize + chunk_len;
// Special case the IEND chunk, which MUST appear last and libpng stops parsing once it hits
// such a chunk (let's do the same).
if (chunk_type == kPngChunkIEND) {
// Truncate the data to the end of this chunk. There may be garbage trailing after
// (b/38169876)
data_ = data_.substr(0, window_end_);
break;
}
} else {
// We want to strip this chunk. If we accumulated a window,
// we must return the window now.
@ -145,14 +162,14 @@ bool PngChunkFilter::Next(const void** buffer, size_t* len) {
}
void PngChunkFilter::BackUp(size_t count) {
if (error_) {
if (HadError()) {
return;
}
window_start_ -= count;
}
bool PngChunkFilter::Rewind() {
if (error_) {
if (HadError()) {
return false;
}
window_start_ = 0;

28
tools/aapt2/compile/PngCrunch.cpp
View File

@ -73,6 +73,11 @@ static void LogWarning(png_structp png_ptr, png_const_charp warning_msg) {
static void LogError(png_structp png_ptr, png_const_charp error_msg) {
IDiagnostics* diag = (IDiagnostics*)png_get_error_ptr(png_ptr);
diag->Error(DiagMessage() << error_msg);
// Causes libpng to longjmp to the spot where setjmp was set. This is how libpng does
// error handling. If this custom error handler method were to return, libpng would, by
// default, print the error message to stdout and call the same png_longjmp method.
png_longjmp(png_ptr, 1);
}
static void ReadDataFromStream(png_structp png_ptr, png_bytep buffer, png_size_t len) {
@ -82,7 +87,12 @@ static void ReadDataFromStream(png_structp png_ptr, png_bytep buffer, png_size_t
size_t in_len;
if (!in->Next(&in_buffer, &in_len)) {
if (in->HadError()) {
std::string err = in->GetError();
std::stringstream error_msg_builder;
error_msg_builder << "failed reading from input";
if (!in->GetError().empty()) {
error_msg_builder << ": " << in->GetError();
}
std::string err = error_msg_builder.str();
png_error(png_ptr, err.c_str());
}
return;
@ -103,6 +113,11 @@ static void WriteDataToStream(png_structp png_ptr, png_bytep buffer, png_size_t
while (len > 0) {
if (!out->Next(&out_buffer, &out_len)) {
if (out->HadError()) {
std::stringstream err_msg_builder;
err_msg_builder << "failed writing to output";
if (!out->GetError().empty()) {
err_msg_builder << ": " << out->GetError();
}
std::string err = out->GetError();
png_error(png_ptr, err.c_str());
}
@ -126,7 +141,7 @@ static void WriteDataToStream(png_structp png_ptr, png_bytep buffer, png_size_t
}
}
std::unique_ptr<Image> ReadPng(IAaptContext* context, io::InputStream* in) {
std::unique_ptr<Image> ReadPng(IAaptContext* context, const Source& source, io::InputStream* in) {
// Read the first 8 bytes of the file looking for the PNG signature.
// Bail early if it does not match.
const png_byte* signature;
@ -163,6 +178,9 @@ std::unique_ptr<Image> ReadPng(IAaptContext* context, io::InputStream* in) {
return {};
}
// Create a diagnostics that has the source information encoded.
SourcePathDiagnostics source_diag(source, context->GetDiagnostics());
// Automatically release PNG resources at end of scope.
PngReadStructDeleter png_read_deleter(read_ptr, info_ptr);
@ -174,7 +192,7 @@ std::unique_ptr<Image> ReadPng(IAaptContext* context, io::InputStream* in) {
}
// Handle warnings ourselves via IDiagnostics.
png_set_error_fn(read_ptr, (png_voidp)context->GetDiagnostics(), LogError, LogWarning);
png_set_error_fn(read_ptr, (png_voidp)&source_diag, LogError, LogWarning);
// Set up the read functions which read from our custom data sources.
png_set_read_fn(read_ptr, (png_voidp)in, ReadDataFromStream);
@ -231,8 +249,8 @@ std::unique_ptr<Image> ReadPng(IAaptContext* context, io::InputStream* in) {
// something
// that can always be represented by 9-patch.
if (width > std::numeric_limits<int32_t>::max() || height > std::numeric_limits<int32_t>::max()) {
context->GetDiagnostics()->Error(
DiagMessage() << "PNG image dimensions are too large: " << width << "x" << height);
source_diag.Error(DiagMessage()
<< "PNG image dimensions are too large: " << width << "x" << height);
return {};
}