From 051b5d9df1f4669117937b7a40104404cc252993 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz=20M=C3=A1s?= Date: Wed, 20 Mar 2019 22:50:14 +0100 Subject: [PATCH 1/5] Fix #742 by detecting incorrect subBox size (cherry picked from commit 1bdd3eab5ebdde324dbfecc3fb6d6495b32d2e4d) --- src/jp2image.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/jp2image.cpp b/src/jp2image.cpp index 39a6f581..a4b2d901 100644 --- a/src/jp2image.cpp +++ b/src/jp2image.cpp @@ -501,8 +501,8 @@ namespace Exiv2 subBox.length = getLong((byte*)&subBox.length, bigEndian); subBox.type = getLong((byte*)&subBox.type, bigEndian); - // subBox.length makes no sense if it is larger than the rest of the file - if (subBox.length > io_->size() - io_->tell()) { + // subBox.length makes no sense if it is larger than the rest of the file || 0 + if (subBox.length == 0 || subBox.length > io_->size() - io_->tell()) { throw Error(kerCorruptedMetadata); } DataBuf data(subBox.length-sizeof(box)); From 8c81e1146c943eb58db536b37b110ff0efcdc523 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz=20M=C3=A1s?= Date: Wed, 20 Mar 2019 22:51:22 +0100 Subject: [PATCH 2/5] clang-format Jp2Image::printStructure() (cherry picked from commit b6e4ca0a8cfff1ad0f6040901382863cccee33cb) # Conflicts: # src/jp2image.cpp --- src/jp2image.cpp | 176 +++++++++++++++++++++++++++-------------------- 1 file changed, 102 insertions(+), 74 deletions(-) diff --git a/src/jp2image.cpp b/src/jp2image.cpp index a4b2d901..92c583b3 100644 --- a/src/jp2image.cpp +++ b/src/jp2image.cpp @@ -446,27 +446,30 @@ namespace Exiv2 } // Jp2Image::readMetadata - void Jp2Image::printStructure(std::ostream& out, PrintStructureOption option,int depth) + void Jp2Image::printStructure(std::ostream& out, PrintStructureOption option, int depth) { - if (io_->open() != 0) throw Error(kerDataSourceOpenFailed, io_->path(), strError()); + if (io_->open() != 0) + throw Error(kerDataSourceOpenFailed, io_->path(), strError()); // Ensure that this is the correct image type if (!isJp2Type(*io_, false)) { - if (io_->error() || io_->eof()) throw Error(kerFailedToReadImageData); + if (io_->error() || io_->eof()) + throw Error(kerFailedToReadImageData); throw Error(kerNotAJpeg); } - bool bPrint = option == kpsBasic || option==kpsRecursive; + bool bPrint = option == kpsBasic || option == kpsRecursive; bool bRecursive = option == kpsRecursive; - bool bICC = option == kpsIccProfile; - bool bXMP = option == kpsXMP; + bool bICC = option == kpsIccProfile; + bool bXMP = option == kpsXMP; bool bIPTCErase = option == kpsIptcErase; - if ( bPrint ) { + if (bPrint) { out << "STRUCTURE OF JPEG2000 FILE: " << io_->path() << std::endl; - out << " address | length | box | data" << std::endl ; + out << " address | length | box | data" << std::endl; } +<<<<<<< HEAD if ( bPrint || bXMP || bICC || bIPTCErase ) { long position = 0; @@ -478,91 +481,108 @@ namespace Exiv2 while (box.length && box.type != kJp2BoxTypeClose && io_->read((byte*)&box, sizeof(box)) == sizeof(box)) { position = io_->tell(); +======= + if (bPrint || bXMP || bICC || bIPTCErase) { + Jp2BoxHeader box = {1, 1}; + Jp2BoxHeader subBox = {1, 1}; + Jp2UuidBox uuid = {{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}}; + bool bLF = false; + + while (box.length && box.type != kJp2BoxTypeClose && io_->read((byte*)&box, sizeof(box)) == sizeof(box)) { + long position = io_->tell(); +>>>>>>> b6e4ca0a8... clang-format Jp2Image::printStructure() box.length = getLong((byte*)&box.length, bigEndian); - box.type = getLong((byte*)&box.type, bigEndian); - - if ( bPrint ) { - out << Internal::stringFormat("%8ld | %8ld | ",(size_t)(position-sizeof(box)),(size_t) box.length) << toAscii(box.type) << " | " ; - bLF = true ; - if ( box.type == kJp2BoxTypeClose ) lf(out,bLF); + box.type = getLong((byte*)&box.type, bigEndian); + + if (bPrint) { + out << Internal::stringFormat("%8ld | %8ld | ", (size_t)(position - sizeof(box)), + (size_t)box.length) + << toAscii(box.type) << " | "; + bLF = true; + if (box.type == kJp2BoxTypeClose) + lf(out, bLF); } - if ( box.type == kJp2BoxTypeClose ) break; + if (box.type == kJp2BoxTypeClose) + break; - switch(box.type) - { - case kJp2BoxTypeJp2Header: - { - lf(out,bLF); + switch (box.type) { + case kJp2BoxTypeJp2Header: { + lf(out, bLF); - while (io_->read((byte*)&subBox, sizeof(subBox)) == sizeof(subBox) - && io_->tell() < position + (long) box.length) // don't read beyond the box! + while (io_->read((byte*)&subBox, sizeof(subBox)) == sizeof(subBox) && + io_->tell() < position + (long)box.length) // don't read beyond the box! { int address = io_->tell() - sizeof(subBox); subBox.length = getLong((byte*)&subBox.length, bigEndian); - subBox.type = getLong((byte*)&subBox.type, bigEndian); + subBox.type = getLong((byte*)&subBox.type, bigEndian); // subBox.length makes no sense if it is larger than the rest of the file || 0 if (subBox.length == 0 || subBox.length > io_->size() - io_->tell()) { throw Error(kerCorruptedMetadata); } - DataBuf data(subBox.length-sizeof(box)); - io_->read(data.pData_,data.size_); - if ( bPrint ) { - out << Internal::stringFormat("%8ld | %8ld | sub:",(size_t)address,(size_t)subBox.length) - << toAscii(subBox.type) - <<" | " << Internal::binaryToString(makeSlice(data, 0, 30)); + DataBuf data(subBox.length - sizeof(box)); + io_->read(data.pData_, data.size_); + if (bPrint) { + out << Internal::stringFormat("%8ld | %8ld | sub:", (size_t)address, + (size_t)subBox.length) + << toAscii(subBox.type) << " | " + << Internal::binaryToString(makeSlice(data, 0, 30)); bLF = true; } - if(subBox.type == kJp2BoxTypeColorHeader) - { - long pad = 3 ; // don't know why there are 3 padding bytes - if ( bPrint ) { - out << " | pad:" ; - for ( int i = 0 ; i < 3 ; i++ ) out<< " " << (int) data.pData_[i]; + if (subBox.type == kJp2BoxTypeColorHeader) { + long pad = 3; // don't know why there are 3 padding bytes + if (bPrint) { + out << " | pad:"; + for (int i = 0; i < 3; i++) + out << " " << (int)data.pData_[i]; } - long iccLength = getULong(data.pData_+pad, bigEndian); - if ( bPrint ) { - out << " | iccLength:" << iccLength ; + long iccLength = getULong(data.pData_ + pad, bigEndian); + if (bPrint) { + out << " | iccLength:" << iccLength; } - if ( bICC ) { - out.write((const char*)data.pData_+pad,iccLength); + if (bICC) { + out.write((const char*)data.pData_ + pad, iccLength); } } - lf(out,bLF); + lf(out, bLF); } } break; - case kJp2BoxTypeUuid: - { - - if (io_->read((byte*)&uuid, sizeof(uuid)) == sizeof(uuid)) - { - bool bIsExif = memcmp(uuid.uuid, kJp2UuidExif, sizeof(uuid))==0; - bool bIsIPTC = memcmp(uuid.uuid, kJp2UuidIptc, sizeof(uuid))==0; - bool bIsXMP = memcmp(uuid.uuid, kJp2UuidXmp , sizeof(uuid))==0; - - bool bUnknown= ! (bIsExif || bIsIPTC || bIsXMP); - - if ( bPrint ) { - if ( bIsExif ) out << "Exif: " ; - if ( bIsIPTC ) out << "IPTC: " ; - if ( bIsXMP ) out << "XMP : " ; - if ( bUnknown) out << "????: " ; + case kJp2BoxTypeUuid: { + if (io_->read((byte*)&uuid, sizeof(uuid)) == sizeof(uuid)) { + bool bIsExif = memcmp(uuid.uuid, kJp2UuidExif, sizeof(uuid)) == 0; + bool bIsIPTC = memcmp(uuid.uuid, kJp2UuidIptc, sizeof(uuid)) == 0; + bool bIsXMP = memcmp(uuid.uuid, kJp2UuidXmp, sizeof(uuid)) == 0; + + bool bUnknown = !(bIsExif || bIsIPTC || bIsXMP); + + if (bPrint) { + if (bIsExif) + out << "Exif: "; + if (bIsIPTC) + out << "IPTC: "; + if (bIsXMP) + out << "XMP : "; + if (bUnknown) + out << "????: "; } DataBuf rawData; - rawData.alloc(box.length-sizeof(uuid)-sizeof(box)); - long bufRead = io_->read(rawData.pData_, rawData.size_); - if (io_->error()) throw Error(kerFailedToReadImageData); - if (bufRead != rawData.size_) throw Error(kerInputDataReadFailed); - - if ( bPrint ){ - out << Internal::binaryToString(makeSlice(rawData,0,40)); + rawData.alloc(box.length - sizeof(uuid) - sizeof(box)); + long bufRead = io_->read(rawData.pData_, rawData.size_); + if (io_->error()) + throw Error(kerFailedToReadImageData); + if (bufRead != rawData.size_) + throw Error(kerInputDataReadFailed); + + if (bPrint) { + out << Internal::binaryToString(makeSlice(rawData, 0, 40)); out.flush(); } - lf(out,bLF); + lf(out, bLF); +<<<<<<< HEAD if(bIsExif && bRecursive && rawData.size_ > 0) { if ( (rawData.pData_[0] == rawData.pData_[1]) @@ -570,31 +590,39 @@ namespace Exiv2 ) { BasicIo::AutoPtr p = BasicIo::AutoPtr(new MemIo(rawData.pData_,rawData.size_)); printTiffStructure(*p,out,option,depth); +======= + if (bIsExif && bRecursive && rawData.size_ > 0) { + if ((rawData.pData_[0] == rawData.pData_[1]) && + (rawData.pData_[0] == 'I' || rawData.pData_[0] == 'M')) { + BasicIo::UniquePtr p = BasicIo::UniquePtr(new MemIo(rawData.pData_, rawData.size_)); + printTiffStructure(*p, out, option, depth); +>>>>>>> b6e4ca0a8... clang-format Jp2Image::printStructure() } } - if(bIsIPTC && bRecursive) - { + if (bIsIPTC && bRecursive) { IptcData::printStructure(out, makeSlice(rawData.pData_, 0, rawData.size_), depth); } - if( bIsXMP && bXMP ) - { - out.write((const char*)rawData.pData_,rawData.size_); + if (bIsXMP && bXMP) { + out.write((const char*)rawData.pData_, rawData.size_); } } } break; - default: break; + default: + break; } // Move to the next box. io_->seek(static_cast(position - sizeof(box) + box.length), BasicIo::beg); - if (io_->error()) throw Error(kerFailedToReadImageData); - if ( bPrint ) lf(out,bLF); + if (io_->error()) + throw Error(kerFailedToReadImageData); + if (bPrint) + lf(out, bLF); } } - } // JpegBase::printStructure + } // JpegBase::printStructure void Jp2Image::writeMetadata() { From 25ddbaa6c026bd0483e8faf79739c3cc21854258 Mon Sep 17 00:00:00 2001 From: Luis Diaz Mas Date: Wed, 27 Mar 2019 07:40:45 +0100 Subject: [PATCH 3/5] Make subBox.length check in jp2image.cpp more robust (cherry picked from commit a154b992ccad71a7d95a94cdedb933fa66a51b61) --- src/jp2image.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/jp2image.cpp b/src/jp2image.cpp index 92c583b3..6b0d8ada 100644 --- a/src/jp2image.cpp +++ b/src/jp2image.cpp @@ -516,10 +516,10 @@ namespace Exiv2 subBox.length = getLong((byte*)&subBox.length, bigEndian); subBox.type = getLong((byte*)&subBox.type, bigEndian); - // subBox.length makes no sense if it is larger than the rest of the file || 0 - if (subBox.length == 0 || subBox.length > io_->size() - io_->tell()) { + if (subBox.length < sizeof(box) || subBox.length > io_->size() - io_->tell()) { throw Error(kerCorruptedMetadata); } + DataBuf data(subBox.length - sizeof(box)); io_->read(data.pData_, data.size_); if (bPrint) { From f33d8daaa09d76aaacc4e770a6c44bee603bcc57 Mon Sep 17 00:00:00 2001 From: Luis Diaz Mas Date: Wed, 27 Mar 2019 07:41:23 +0100 Subject: [PATCH 4/5] Add regression test for #742 (cherry picked from commit 885dd2a7437b946c975f2a37c9ccaecc1b91fc95) --- test/data/issue_742_poc | Bin 0 -> 84 bytes tests/bugfixes/github/test_issue_742.py | 14 ++++++++++++++ 2 files changed, 14 insertions(+) create mode 100644 test/data/issue_742_poc create mode 100644 tests/bugfixes/github/test_issue_742.py diff --git a/test/data/issue_742_poc b/test/data/issue_742_poc new file mode 100644 index 0000000000000000000000000000000000000000..6b5eb689b02af1ca9fcd438150022c55a621d1ff GIT binary patch literal 84 zcmZQzVBpCLP*C9IYUg5LU=YbFFv Date: Sun, 31 Mar 2019 09:41:11 +0200 Subject: [PATCH 5/5] Update tests/bugfixes/github/test_issue_742.py Co-Authored-By: piponazo (cherry picked from commit 39d8904696338d5bd4a9c7e9a96a798a791d0973) --- src/jp2image.cpp | 23 +---------------------- tests/bugfixes/github/test_issue_742.py | 2 +- 2 files changed, 2 insertions(+), 23 deletions(-) diff --git a/src/jp2image.cpp b/src/jp2image.cpp index 6b0d8ada..7c270486 100644 --- a/src/jp2image.cpp +++ b/src/jp2image.cpp @@ -469,7 +469,6 @@ namespace Exiv2 out << " address | length | box | data" << std::endl; } -<<<<<<< HEAD if ( bPrint || bXMP || bICC || bIPTCErase ) { long position = 0; @@ -481,16 +480,6 @@ namespace Exiv2 while (box.length && box.type != kJp2BoxTypeClose && io_->read((byte*)&box, sizeof(box)) == sizeof(box)) { position = io_->tell(); -======= - if (bPrint || bXMP || bICC || bIPTCErase) { - Jp2BoxHeader box = {1, 1}; - Jp2BoxHeader subBox = {1, 1}; - Jp2UuidBox uuid = {{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}}; - bool bLF = false; - - while (box.length && box.type != kJp2BoxTypeClose && io_->read((byte*)&box, sizeof(box)) == sizeof(box)) { - long position = io_->tell(); ->>>>>>> b6e4ca0a8... clang-format Jp2Image::printStructure() box.length = getLong((byte*)&box.length, bigEndian); box.type = getLong((byte*)&box.type, bigEndian); @@ -582,21 +571,11 @@ namespace Exiv2 } lf(out, bLF); -<<<<<<< HEAD - if(bIsExif && bRecursive && rawData.size_ > 0) - { - if ( (rawData.pData_[0] == rawData.pData_[1]) - && (rawData.pData_[0]=='I' || rawData.pData_[0]=='M' ) - ) { - BasicIo::AutoPtr p = BasicIo::AutoPtr(new MemIo(rawData.pData_,rawData.size_)); - printTiffStructure(*p,out,option,depth); -======= if (bIsExif && bRecursive && rawData.size_ > 0) { if ((rawData.pData_[0] == rawData.pData_[1]) && (rawData.pData_[0] == 'I' || rawData.pData_[0] == 'M')) { - BasicIo::UniquePtr p = BasicIo::UniquePtr(new MemIo(rawData.pData_, rawData.size_)); + BasicIo::AutoPtr p = BasicIo::AutoPtr(new MemIo(rawData.pData_, rawData.size_)); printTiffStructure(*p, out, option, depth); ->>>>>>> b6e4ca0a8... clang-format Jp2Image::printStructure() } } diff --git a/tests/bugfixes/github/test_issue_742.py b/tests/bugfixes/github/test_issue_742.py index 6aa4a339..363e616b 100644 --- a/tests/bugfixes/github/test_issue_742.py +++ b/tests/bugfixes/github/test_issue_742.py @@ -8,7 +8,7 @@ class ThrowsWhenSubBoxLengthIsNotGood(metaclass=system_tests.CaseMeta): filename = system_tests.path("$data_path/issue_742_poc") commands = ["$exiv2 -pX $filename"] stdout = [""] - stderr = ["""$exiv2_exception_message """ + filename + """: + stderr = ["""$exiv2_exception_message $filename: $kerCorruptedMetadata """] retval = [1]