Remove 2nd param from Photoshop::isIrb() since it is always hardcoded

This commit is contained in:
Luis Diaz 2022-04-05 09:58:20 +02:00 committed by Luis Díaz Más
parent 7366a64d44
commit 8ab7800477
4 changed files with 24 additions and 19 deletions

View File

@ -27,11 +27,11 @@ struct EXIV2API Photoshop {
static constexpr uint16_t preview_ = 0x040c; //!< %Photoshop preview marker
/// @brief Checks an IRB
/// @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);
/// @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);
/// @brief Validates all IRBs
/// @param pPsData Existing IRB buffer

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);

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

@ -12,20 +12,23 @@ 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), 4));
ASSERT_TRUE(Photoshop::isIrb(reinterpret_cast<const byte*>(marker)));
}
}
TEST(Photoshop_isIrb, returnsFalseWithInvalidMarkers) {
ASSERT_FALSE(Photoshop::isIrb(reinterpret_cast<const byte *>("7BIM"), 4));
ASSERT_FALSE(Photoshop::isIrb(reinterpret_cast<const byte *>("AGHg"), 4));
ASSERT_FALSE(Photoshop::isIrb(reinterpret_cast<const byte *>("dcsr"), 4));
ASSERT_FALSE(Photoshop::isIrb(reinterpret_cast<const byte *>("LUIS"), 4));
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, returnsFalseWithInvalidSize) {
ASSERT_FALSE(Photoshop::isIrb(reinterpret_cast<const byte *>("8BIM"), 3));
ASSERT_FALSE(Photoshop::isIrb(reinterpret_cast<const byte *>("8BIM"), 0));
ASSERT_FALSE(Photoshop::isIrb(reinterpret_cast<const byte *>("8BIM"), 5));
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")));
}