Change logic to determine if segment has size

Note that the failing tests that had to be adapted were bad formed files
from FUZZERs. We should not consider invalid markers like 0x00 or 0x52
but only undefined APPn markers.
This commit is contained in:
Luis Diaz 2022-04-04 17:59:31 +02:00 committed by Luis Díaz Más
parent 400632f27b
commit 047f6b733e
5 changed files with 29 additions and 39 deletions

View File

@ -150,7 +150,6 @@ class EXIV2API JpegBase : public Image {
virtual int writeHeader(BasicIo& oIo) const = 0;
//@}
private:
//! @name Manipulators
//@{
@ -246,7 +245,7 @@ class EXIV2API JpegImage : public JpegBase {
private:
// Constant data
static const byte blank_[]; ///< Minimal Jpeg image
static const byte blank_[]; ///< Minimal Jpeg image
};
//! Helper class to access %Exiv2 files

View File

@ -29,41 +29,30 @@
namespace Exiv2 {
namespace {
// JPEG Segment markers (The first byte is always 0xFF, the value of these constants correspond to the 2nd byte)
constexpr byte dht_ = 0xc4; //!< JPEG DHT marker: Define Huffman Table(s)
constexpr byte dqt_ = 0xdb; //!< JPEG DQT marker: Define Quantization Table(s)
constexpr byte dri_ = 0xdd; //!< JPEG DRI marker
constexpr byte sos_ = 0xda; //!< JPEG SOS marker
constexpr byte eoi_ = 0xd9; //!< JPEG EOI marker
constexpr byte app0_ = 0xe0; //!< JPEG APP0 marker
constexpr byte app1_ = 0xe1; //!< JPEG APP1 marker
constexpr byte app2_ = 0xe2; //!< JPEG APP2 marker
constexpr byte app13_ = 0xed; //!< JPEG APP13 marker
constexpr byte com_ = 0xfe; //!< JPEG Comment marker
constexpr byte soi_ = 0xd8; ///!< SOI marker
// Markers without payload
constexpr byte soi_ = 0xd8; ///!< SOI marker
constexpr byte eoi_ = 0xd9; //!< JPEG EOI marker
constexpr byte rst1_ = 0xd0; //!< JPEG Restart 0 Marker (from 0xD0 to 0xD7 there might be 8 of these markers)
// Start of Frame markers, nondifferential Huffman-coding frames
constexpr byte sof0_ = 0xc0; //!< JPEG Start-Of-Frame marker
constexpr byte sof1_ = 0xc1; //!< JPEG Start-Of-Frame marker
constexpr byte sof2_ = 0xc2; //!< JPEG Start-Of-Frame marker
constexpr byte sof3_ = 0xc3; //!< JPEG Start-Of-Frame marker
// Start of Frame markers, differential Huffman-coding frames
constexpr byte sof5_ = 0xc5; //!< JPEG Start-Of-Frame marker
constexpr byte sof6_ = 0xc6; //!< JPEG Start-Of-Frame marker
constexpr byte sof7_ = 0xc7; //!< JPEG Start-Of-Frame marker
// Start of Frame markers, nondifferential arithmetic-coding frames
constexpr byte sof9_ = 0xc9; //!< JPEG Start-Of-Frame marker
constexpr byte sof10_ = 0xca; //!< JPEG Start-Of-Frame marker
constexpr byte sof11_ = 0xcb; //!< JPEG Start-Of-Frame marker
// Start of Frame markers, differential arithmetic-coding frames
constexpr byte sof13_ = 0xcd; //!< JPEG Start-Of-Frame marker
constexpr byte sof14_ = 0xce; //!< JPEG Start-Of-Frame marker
constexpr byte sof15_ = 0xcf; //!< JPEG Start-Of-Frame marker
constexpr auto exifId_ = "Exif\0\0"; //!< Exif identifier
constexpr auto jfifId_ = "JFIF\0"; //!< JFIF identifier
constexpr auto exifId_ = "Exif\0\0"; //!< Exif identifier
// constexpr auto jfifId_ = "JFIF\0"; //!< JFIF identifier
constexpr auto xmpId_ = "http://ns.adobe.com/xap/1.0/\0"; //!< XMP packet identifier
constexpr auto iccId_ = "ICC_PROFILE\0"; //!< ICC profile identifier
@ -76,12 +65,11 @@ inline bool inRange2(int value, int lo1, int hi1, int lo2, int hi2) {
}
/// @brief has the segment a non-zero payload?
/// @param marker The marker at the start of a segment
/// @param m The marker at the start of a segment
/// @return true if the segment has a length field/payload
bool markerHasLength(byte marker) {
/// \todo there are less markers without payload. Maybe we should revert the logic.
return (marker >= sof0_ && marker <= sof15_) || (marker >= app0_ && marker <= (app0_ | 0x0F)) || marker == dht_ ||
marker == dqt_ || marker == dri_ || marker == com_ || marker == sos_;
bool markerHasLength(byte m) {
bool markerWithoutLength = m >= rst1_ && m <= eoi_;
return !markerWithoutLength;
}
void readSegmentSize(const byte marker, BasicIo& io, std::array<byte, 2>& buf, std::uint16_t& size) {
@ -294,6 +282,7 @@ byte JpegBase::advanceToMarker(ErrorCode err) const {
throw Error(err);
}
/// \todo should we check for validity of the marker? 0x59 does not look fine
// Markers can start with any number of 0xff
while ((c = io_->getb()) == 0xff) {
}

View File

@ -11,7 +11,9 @@ class TiffMnEntryDoCountInvalidTiffType(metaclass=CaseMeta):
filename = path("$data_path/issue_1833_poc.jpg")
commands = ["$exiv2 -pS $filename"]
stderr = [""]
retval = [0]
stderr = ["""$exiv2_exception_message """ + filename + """:
$kerFailedToReadImageData
"""]
retval = [1]
compare_stdout = check_no_ASAN_UBSAN_errors

View File

@ -1,6 +1,6 @@
# -*- coding: utf-8 -*-
from system_tests import CaseMeta, CopyTmpFiles, path
from system_tests import CaseMeta, CopyTmpFiles, path, check_no_ASAN_UBSAN_errors
@CopyTmpFiles("$data_path/issue_1881_poc.jpg", "$data_path/issue_1881_coverage.jpg")
class SonyPreviewImageLargeAllocation(metaclass=CaseMeta):
@ -13,6 +13,9 @@ class SonyPreviewImageLargeAllocation(metaclass=CaseMeta):
filename1 = path("$tmp_path/issue_1881_poc.jpg")
filename2 = path("$tmp_path/issue_1881_coverage.jpg")
commands = ["$exiv2 -q -d I rm $filename1", "$exiv2 -q -d I rm $filename2"]
stdout = ["",""]
stderr = ["",""]
retval = [0,0]
stderr = ["""$exception_in_erase """ + filename1 + """:
$kerFailedToReadImageData
""", ""]
retval = [1,0]
compare_stdout = check_no_ASAN_UBSAN_errors

View File

@ -1,6 +1,5 @@
import system_tests
class BufferOverReadInNikon1MakerNotePrint0x0088(
metaclass=system_tests.CaseMeta):
@ -9,12 +8,10 @@ class BufferOverReadInNikon1MakerNotePrint0x0088(
filename = system_tests.path(
"$data_path/NikonMakerNotePrint0x088_overread"
)
commands = ["$exiv2 -pt --grep AFFocusPos $filename"]
stdout = [
"""Exif.Nikon1.AFFocusPos Undefined 4 Invalid value; Center
"""
]
stderr = [""]
retval = [0]
commands = ["$exiv2 -q -pt --grep AFFocusPos $filename"]
stderr = ["""$exiv2_exception_message """ + filename + """:
$kerFailedToReadImageData
"""]
retval = [1]
compare_stderr = system_tests.check_no_ASAN_UBSAN_errors