Merge pull request #2181 from Exiv2/main_issue2179

[main] Fix integer overflow #2179
This commit is contained in:
Luis Díaz Más 2022-04-06 16:27:07 +02:00 committed by GitHub
commit 235bf522b2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 229 additions and 67 deletions

View File

@ -18,77 +18,58 @@ namespace Exiv2 {
// *****************************************************************************
// class definitions
/*!
@brief Helper class, has methods to deal with %Photoshop "Information
Resource Blocks" (IRBs).
*/
/// @brief Helper class, has methods to deal with %Photoshop "Information Resource Blocks" (IRBs).
struct EXIV2API Photoshop {
// Todo: Public for now
static constexpr std::array irbId_{"8BIM", "AgHg", "DCSR", "PHUT"}; //!< %Photoshop IRB markers
static constexpr auto ps3Id_ = "Photoshop 3.0\0"; //!< %Photoshop marker
static constexpr auto bimId_ = "8BIM"; //!< %Photoshop IRB marker (deprecated)
static constexpr uint16_t iptc_ = 0x0404; //!< %Photoshop IPTC marker
static constexpr uint16_t preview_ = 0x040c; //!< %Photoshop preview marker
/*!
@brief Checks an IRB
/// @brief Checks an IRB
/// @param pPsData Existing IRB buffer. It is expected to be of size 4.
/// @return true if the IRB marker is known
/// @todo This should be an implementation detail and not exposed in the API. An attacker could try to pass
/// a smaller buffer or null pointer.
static bool isIrb(const byte* pPsData);
@param pPsData Existing IRB buffer
@param sizePsData Size of the IRB buffer
@return true if the IRB marker is known and the buffer is big enough to check this;<BR>
false otherwise
*/
static bool isIrb(const byte* pPsData, size_t sizePsData);
/*!
@brief Validates all IRBs
@param pPsData Existing IRB buffer
@param sizePsData Size of the IRB buffer, may be 0
@return true if all IRBs are valid;<BR>
false otherwise
*/
/// @brief Validates all IRBs
/// @param pPsData Existing IRB buffer
/// @param sizePsData Size of the IRB buffer, may be 0
/// @return true if all IRBs are valid;<BR> false otherwise
static bool valid(const byte* pPsData, size_t sizePsData);
/*!
@brief Locates the data for a %Photoshop tag in a %Photoshop formated memory
buffer. Operates on raw data to simplify reuse.
@param pPsData Pointer to buffer containing entire payload of
%Photoshop formated data, e.g., from APP13 Jpeg segment.
@param sizePsData Size in bytes of pPsData.
@param psTag %Tag number of the block to look for.
@param record Output value that is set to the start of the
data block within pPsData (may not be null).
@param sizeHdr Output value that is set to the size of the header
within the data block pointed to by record (may not be null).
@param sizeData Output value that is set to the size of the actual
data within the data block pointed to by record (may not be null).
@return 0 if successful;<BR>
3 if no data for psTag was found in pPsData;<BR>
-2 if the pPsData buffer does not contain valid data.
*/
/// @brief Locates the data for a %Photoshop tag in a %Photoshop formated memory buffer.
/// Operates on raw data to simplify reuse.
/// @param pPsData Pointer to buffer containing entire payload of %Photoshop formated data (from APP13 Jpeg segment)
/// @param sizePsData Size in bytes of pPsData.
/// @param psTag %Tag number of the block to look for.
/// @param record Output value that is set to the start of the data block within pPsData (may not be null).
/// @param sizeHdr Output value that is set to the size of the header within the data block pointed to by record
/// (may not be null).
/// @param sizeData Output value that is set to the size of the actual data within the data block pointed to by record
/// (may not be null).
/// @return 0 if successful;<BR>
/// 3 if no data for psTag was found in pPsData;<BR>
/// -2 if the pPsData buffer does not contain valid data.
static int locateIrb(const byte* pPsData, size_t sizePsData, uint16_t psTag, const byte** record,
uint32_t* const sizeHdr, uint32_t* const sizeData);
/*!
@brief Forwards to locateIrb() with \em psTag = \em iptc_
*/
/// @brief Forwards to locateIrb() with \em psTag = \em iptc_
static int locateIptcIrb(const byte* pPsData, size_t sizePsData, const byte** record, uint32_t* const sizeHdr,
uint32_t* const sizeData);
/*!
@brief Forwards to locatePreviewIrb() with \em psTag = \em preview_
*/
/// @brief Forwards to locatePreviewIrb() with \em psTag = \em preview_
static int locatePreviewIrb(const byte* pPsData, size_t sizePsData, const byte** record, uint32_t* const sizeHdr,
uint32_t* const sizeData);
/*!
@brief Set the new IPTC IRB, keeps existing IRBs but removes the
IPTC block if there is no new IPTC data to write.
@param pPsData Existing IRB buffer
@param sizePsData Size of the IRB buffer, may be 0
@param iptcData Iptc data to embed, may be empty
@return A data buffer containing the new IRB buffer, may have 0 size
*/
/// @brief Set the new IPTC IRB, keeps existing IRBs but removes the IPTC block if there is no new IPTC data to write.
/// @param pPsData Existing IRB buffer
/// @param sizePsData Size of the IRB buffer, may be 0
/// @param iptcData Iptc data to embed, may be empty
/// @return A data buffer containing the new IRB buffer, may have 0 size
static DataBuf setIptcIrb(const byte* pPsData, size_t sizePsData, const IptcData& iptcData);
}; // class Photoshop
};
/*!
@brief Abstract helper base class to access JPEG images.

View File

@ -35,9 +35,11 @@ static inline bool inRange2(int value, int lo1, int hi1, int lo2, int hi2) {
return inRange(lo1, value, hi1) || inRange(lo2, value, hi2);
}
bool Photoshop::isIrb(const byte* pPsData, size_t sizePsData) {
if (sizePsData < 4)
bool Photoshop::isIrb(const byte* pPsData) {
if (pPsData == nullptr) {
return false;
}
/// \todo check if direct array comparison is faster than a call to memcmp
return std::any_of(irbId_.begin(), irbId_.end(), [pPsData](auto id) { return memcmp(pPsData, id, 4) == 0; });
}
@ -69,7 +71,7 @@ int Photoshop::locateIrb(const byte* pPsData, size_t sizePsData, uint16_t psTag,
std::cerr << "Photoshop::locateIrb: ";
#endif
// Data should follow Photoshop format, if not exit
while (position <= sizePsData - 12 && isIrb(pPsData + position, 4)) {
while (position <= (sizePsData - 12) && isIrb(pPsData + position)) {
const byte* hrd = pPsData + position;
position += 4;
uint16_t type = getUShort(pPsData + position, bigEndian);
@ -150,16 +152,17 @@ DataBuf Photoshop::setIptcIrb(const byte* pPsData, size_t sizePsData, const Iptc
uint32_t sizeIptc = 0;
uint32_t sizeHdr = 0;
DataBuf rc;
// Safe to call with zero psData.size_
if (0 > Photoshop::locateIptcIrb(pPsData, sizePsData, &record, &sizeHdr, &sizeIptc)) {
return rc;
}
Blob psBlob;
const auto sizeFront = static_cast<size_t>(record - pPsData);
// Write data before old record.
if (sizePsData > 0 && sizeFront > 0) {
append(psBlob, pPsData, sizeFront);
}
// Write new iptc record if we have it
DataBuf rawIptc = IptcParser::encode(iptcData);
if (!rawIptc.empty()) {
@ -175,21 +178,24 @@ DataBuf Photoshop::setIptcIrb(const byte* pPsData, size_t sizePsData, const Iptc
if (rawIptc.size() & 1)
psBlob.push_back(0x00);
}
// Write existing stuff after record,
// skip the current and all remaining IPTC blocks
// Write existing stuff after record, skip the current and all remaining IPTC blocks
size_t pos = sizeFront;
while (0 == Photoshop::locateIptcIrb(pPsData + pos, sizePsData - pos, &record, &sizeHdr, &sizeIptc)) {
long nextSizeData = Safe::add<long>(static_cast<long>(sizePsData), -static_cast<long>(pos));
enforce(nextSizeData >= 0, ErrorCode::kerCorruptedMetadata);
while (0 == Photoshop::locateIptcIrb(pPsData + pos, nextSizeData, &record, &sizeHdr, &sizeIptc)) {
const auto newPos = static_cast<size_t>(record - pPsData);
// Copy data up to the IPTC IRB
if (newPos > pos) {
if (newPos > pos) { // Copy data up to the IPTC IRB
append(psBlob, pPsData + pos, newPos - pos);
}
// Skip the IPTC IRB
pos = newPos + sizeHdr + sizeIptc + (sizeIptc & 1);
pos = newPos + sizeHdr + sizeIptc + (sizeIptc & 1); // Skip the IPTC IRB
nextSizeData = Safe::add<long>(static_cast<long>(sizePsData), -static_cast<long>(pos));
enforce(nextSizeData >= 0, ErrorCode::kerCorruptedMetadata);
}
if (pos < sizePsData) {
append(psBlob, pPsData + pos, sizePsData - pos);
}
// Data is rounded to be even
if (!psBlob.empty())
rc = DataBuf(&psBlob[0], psBlob.size());

View File

@ -175,7 +175,7 @@ void PsdImage::readMetadata() {
throw Error(ErrorCode::kerNotAnImage, "Photoshop");
}
if (!Photoshop::isIrb(buf, 4)) {
if (!Photoshop::isIrb(buf)) {
break; // bad resource type
}
uint16_t resourceId = getUShort(buf + 4, bigEndian);
@ -413,7 +413,7 @@ void PsdImage::doWriteMetadata(BasicIo& outIo) {
// read resource type and ID
uint32_t resourceType = getULong(buf, bigEndian);
if (!Photoshop::isIrb(buf, 4)) {
if (!Photoshop::isIrb(buf)) {
throw Error(ErrorCode::kerNotAnImage, "Photoshop"); // bad resource type
}
uint16_t resourceId = getUShort(buf + 4, bigEndian);

View File

@ -18,6 +18,7 @@ add_executable(unit_tests
test_jp2image_int.cpp
test_IptcKey.cpp
test_LangAltValueRead.cpp
test_Photoshop.cpp
test_pngimage.cpp
test_safe_op.cpp
test_slice.cpp

View File

@ -0,0 +1,174 @@
// SPDX-License-Identifier: GPL-2.0-or-later
#include <exiv2/jpgimage.hpp>
#include <gtest/gtest.h>
using namespace Exiv2;
namespace {
constexpr std::array validMarkers{"8BIM", "AgHg", "DCSR", "PHUT"};
}
TEST(Photoshop_isIrb, returnsTrueWithValidMarkers) {
for (const auto& marker : validMarkers) {
ASSERT_TRUE(Photoshop::isIrb(reinterpret_cast<const byte*>(marker)));
}
}
TEST(Photoshop_isIrb, returnsFalseWithInvalidMarkers) {
ASSERT_FALSE(Photoshop::isIrb(reinterpret_cast<const byte*>("7BIM")));
ASSERT_FALSE(Photoshop::isIrb(reinterpret_cast<const byte*>("AGHg")));
ASSERT_FALSE(Photoshop::isIrb(reinterpret_cast<const byte*>("dcsr")));
ASSERT_FALSE(Photoshop::isIrb(reinterpret_cast<const byte*>("LUIS")));
}
TEST(Photoshop_isIrb, returnsFalseWithNullPointer) {
ASSERT_FALSE(Photoshop::isIrb(nullptr));
}
/// \note probably this is not safe and we need to remove it
TEST(Photoshop_isIrb, returnsFalseWithShorterMarker) {
ASSERT_FALSE(Photoshop::isIrb(reinterpret_cast<const byte*>("8BI")));
}
TEST(Photoshop_locateIrb, returnsMinus2withInvalidPhotoshopIRB) {
const std::string data{"8BIMlalalalalalala"};
const Exiv2::byte* record;
uint32_t sizeHdr = 0;
uint32_t sizeData = 0;
ASSERT_EQ(-2, Photoshop::locateIrb(reinterpret_cast<const byte*>(data.c_str()), data.size(), Photoshop::iptc_,
&record, &sizeHdr, &sizeData));
}
TEST(Photoshop_locateIrb, returnsMinus2WithMarkerNotStartingWith8BIM) {
const std::string data{"7BIMlalalalalalalala"};
const Exiv2::byte* record;
uint32_t sizeHdr = 0;
uint32_t sizeData = 0;
ASSERT_EQ(-2, Photoshop::locateIrb(reinterpret_cast<const byte*>(data.c_str()), data.size(), Photoshop::iptc_,
&record, &sizeHdr, &sizeData));
}
TEST(Photoshop_locateIrb, returns3withNotLongEnoughData) {
const std::string data{"8BIMlala"};
const Exiv2::byte* record;
uint32_t sizeHdr = 0;
uint32_t sizeData = 0;
ASSERT_EQ(3, Photoshop::locateIrb(reinterpret_cast<const byte*>(data.c_str()), data.size(), Photoshop::iptc_, &record,
&sizeHdr, &sizeData));
}
TEST(Photoshop_locateIrb, returns0withGoodIptcIrb) {
// Data taken from file test/data/DSC_3079.jpg
// The IPTC marker is 0x04 0x04
const std::array<byte, 68> data{0x38, 0x42, 0x49, 0x4d, 0x04, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x1b, 0x1c, 0x01,
0x5a, 0x00, 0x03, 0x1b, 0x25, 0x47, 0x1c, 0x02, 0x00, 0x00, 0x02, 0x00, 0x04, 0x1c,
0x02, 0x19, 0x00, 0x07, 0x41, 0x6d, 0x65, 0x72, 0x69, 0x63, 0x61, 0x00, 0x38, 0x42,
0x49, 0x4d, 0x04, 0x25, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x3f, 0x65, 0x16, 0xda,
0x51, 0x3f, 0xfe, 0x5c, 0xbb, 0x52, 0xf3, 0x2e, 0x36, 0x7b, 0x97, 0x3d};
const Exiv2::byte* record;
uint32_t sizeHdr = 0;
uint32_t sizeData = 0;
ASSERT_EQ(0, Photoshop::locateIrb(data.data(), data.size(), Photoshop::iptc_, &record, &sizeHdr, &sizeData));
ASSERT_EQ(12, sizeHdr);
ASSERT_EQ(27, sizeData);
}
TEST(Photoshop_locateIptcIrb, returns0withGoodIptcIrb) {
// Data taken from file test/data/DSC_3079.jpg
// The IPTC marker is 0x04 0x04
const std::array<byte, 68> data{0x38, 0x42, 0x49, 0x4d, 0x04, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x1b, 0x1c, 0x01,
0x5a, 0x00, 0x03, 0x1b, 0x25, 0x47, 0x1c, 0x02, 0x00, 0x00, 0x02, 0x00, 0x04, 0x1c,
0x02, 0x19, 0x00, 0x07, 0x41, 0x6d, 0x65, 0x72, 0x69, 0x63, 0x61, 0x00, 0x38, 0x42,
0x49, 0x4d, 0x04, 0x25, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x3f, 0x65, 0x16, 0xda,
0x51, 0x3f, 0xfe, 0x5c, 0xbb, 0x52, 0xf3, 0x2e, 0x36, 0x7b, 0x97, 0x3d};
const Exiv2::byte* record;
uint32_t sizeHdr = 0;
uint32_t sizeData = 0;
ASSERT_EQ(0, Photoshop::locateIptcIrb(data.data(), data.size(), &record, &sizeHdr, &sizeData));
ASSERT_EQ(12, sizeHdr);
ASSERT_EQ(27, sizeData);
}
TEST(Photoshop_locateIptcIrb, returns3withoutIptcMarker) {
// Data taken from file test/data/DSC_3079.jpg
// The IPTC marker (0x04 0x04) was manipulated to 0x03 0x04
const std::array<byte, 68> data{0x38, 0x42, 0x49, 0x4d, 0x03, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x1b, 0x1c, 0x01,
0x5a, 0x00, 0x03, 0x1b, 0x25, 0x47, 0x1c, 0x02, 0x00, 0x00, 0x02, 0x00, 0x04, 0x1c,
0x02, 0x19, 0x00, 0x07, 0x41, 0x6d, 0x65, 0x72, 0x69, 0x63, 0x61, 0x00, 0x38, 0x42,
0x49, 0x4d, 0x04, 0x25, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x3f, 0x65, 0x16, 0xda,
0x51, 0x3f, 0xfe, 0x5c, 0xbb, 0x52, 0xf3, 0x2e, 0x36, 0x7b, 0x97, 0x3d};
const Exiv2::byte* record;
uint32_t sizeHdr = 0;
uint32_t sizeData = 0;
ASSERT_EQ(3, Photoshop::locateIptcIrb(data.data(), data.size(), &record, &sizeHdr, &sizeData));
}
TEST(Photoshop_locatePreviewIrb, returns0withGoodPreviewIrb) {
// Data taken from file test/data/DSC_3079.jpg
// The IPTC marker is 0x04 0x04 was transformed to a Preview one => (0x04 0x0c)
const std::array<byte, 68> data{0x38, 0x42, 0x49, 0x4d, 0x04, 0x0c, 0x00, 0x00, 0x00, 0x00, 0x00, 0x1b, 0x1c, 0x01,
0x5a, 0x00, 0x03, 0x1b, 0x25, 0x47, 0x1c, 0x02, 0x00, 0x00, 0x02, 0x00, 0x04, 0x1c,
0x02, 0x19, 0x00, 0x07, 0x41, 0x6d, 0x65, 0x72, 0x69, 0x63, 0x61, 0x00, 0x38, 0x42,
0x49, 0x4d, 0x04, 0x25, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x3f, 0x65, 0x16, 0xda,
0x51, 0x3f, 0xfe, 0x5c, 0xbb, 0x52, 0xf3, 0x2e, 0x36, 0x7b, 0x97, 0x3d};
const Exiv2::byte* record;
uint32_t sizeHdr = 0;
uint32_t sizeData = 0;
ASSERT_EQ(0, Photoshop::locatePreviewIrb(data.data(), data.size(), &record, &sizeHdr, &sizeData));
ASSERT_EQ(12, sizeHdr);
ASSERT_EQ(27, sizeData);
}
// --------------------------------
TEST(Photoshop_setIptcIrb, withEmptyDataReturnsEmptyBuffer) {
const IptcData iptc;
DataBuf buf = Photoshop::setIptcIrb(nullptr, 0, iptc);
ASSERT_TRUE(buf.empty());
}
TEST(Photoshop_setIptcIrb, detectIntegerOverflow_withDataFromPOC2179) {
const std::array<byte, 141> data{
0x38, 0x42, 0x49, 0x4d, 0x20, 0x20, 0x00, 0x20, 0x00, 0x00, 0x00, 0x00, 0x38, 0x42, 0x49, 0x4d, 0x04, 0x04,
0x00, 0x20, 0x00, 0x00, 0x00, 0x75, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0xff, 0x20, 0x20,
0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0xff, 0xff, 0x20, 0x20, 0x20,
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xd9, 0x20, 0xff, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20,
0x20, 0x20, 0x20, 0x20, 0x20, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20,
0xff, 0xff, 0x20, 0x20, 0xff, 0x20, 0xff, 0xff, 0xff, 0xff, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20,
0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0xff, 0xff, 0x20, 0x20, 0x20, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
0xff, 0xff, 0xff, 0x20, 0xff, 0xff, 0xff, 0xff, 0x20, 0xff, 0xff, 0x20, 0xff, 0xff, 0xff};
const IptcData iptc;
ASSERT_THROW(Photoshop::setIptcIrb(data.data(), data.size(), iptc), Exiv2::Error);
}
TEST(Photoshop_setIptcIrb, returnsEmptyBufferWhenDataDoesNotHave8BIM) {
// First byte replaced from 0x38 to 0x37
const std::array<byte, 181> data{
0x37, 0x42, 0x49, 0x4d, 0x20, 0x20, 0x00, 0x20, 0x00, 0x00, 0x00, 0x00, 0x38, 0x42, 0x49, 0x4d, 0x04, 0x04, 0x00,
0x20, 0x00, 0x00, 0x00, 0x75, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0xff, 0x20, 0x20, 0x20, 0x20,
0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0xff, 0xff, 0x20, 0x20, 0x20, 0xff, 0xff, 0xff,
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xd9, 0x20, 0xff, 0x20, 0x20, 0xff, 0xed, 0x00, 0x15, 0x50, 0x68, 0x6f, 0x74,
0x6f, 0x73, 0x68, 0x6f, 0x70, 0x20, 0x33, 0x2e, 0x30, 0x00, 0x20, 0x20, 0x20, 0x20, 0x20, 0xff, 0xed, 0x00, 0x54,
0x50, 0x68, 0x6f, 0x74, 0x6f, 0x73, 0x68, 0x6f, 0x70, 0x20, 0x33, 0x2e, 0x30, 0x00, 0x20, 0x20, 0x20, 0x20, 0xff,
0xff, 0xff, 0xff, 0xff, 0xff, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0xff, 0xff, 0x20, 0x20, 0xff, 0x20, 0xff,
0xff, 0xff, 0xff, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0xff,
0xff, 0x20, 0x20, 0x20, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x20, 0xff, 0xff, 0xff, 0xff, 0x20,
0xff, 0xff, 0x20, 0xff, 0xff, 0xff, 0xff, 0xd9, 0x0d, 0x0a};
const IptcData iptc;
DataBuf buf = Photoshop::setIptcIrb(data.data(), data.size(), iptc);
ASSERT_TRUE(buf.empty());
}