Harden ICC parser

Add missing implicit size checks of tags by passing the already checked
explicitly given size forward.

Also adds my fuzzing test for the ICC parser as it is security critical,
by being used by multiple image formats.

Change-Id: Ieb632ccb78f9b445a276959ffbd66fa04a7a5b45
Reviewed-by: Eirik Aavitsland <eirik.aavitsland@qt.io>

1 files changed, 90 insertions, 52 deletions

diff --git a/src/gui/painting/qicc.cpp b/src/gui/painting/qicc.cpp
index 2abb42993b..53055b426c 100644
--- a/src/gui/painting/qicc.cpp
+++ b/src/gui/painting/qicc.cpp
@@ -190,23 +190,17 @@ static float fromFixedS1516(int x)
     return x * (1.0f / 65536.0f);
 }
 
-QColorVector fromXyzData(const XYZTagData *xyz)
-{
-    const float x = fromFixedS1516(xyz->fixedX);
-    const float y = fromFixedS1516(xyz->fixedY);
-    const float z = fromFixedS1516(xyz->fixedZ);
-    qCDebug(lcIcc) << "XYZ_ " << x << y << z;
-
-    return QColorVector(x, y, z);
-}
-
 static bool isValidIccProfile(const ICCProfileHeader &header)
 {
     if (header.signature != uint(Tag::acsp)) {
         qCWarning(lcIcc, "Failed ICC signature test");
         return false;
     }
-    if (header.profileSize < (sizeof(ICCProfileHeader) + header.tagCount * sizeof(TagTableEntry))) {
+
+    // Don't overflow 32bit integers:
+    if (header.tagCount >= INT32_MAX / sizeof(TagTableEntry))
+        return false;
+    if (header.profileSize - sizeof(ICCProfileHeader) < header.tagCount * sizeof(TagTableEntry)) {
         qCWarning(lcIcc, "Failed basic size sanity");
         return false;
     }
@@ -413,17 +407,44 @@ QByteArray toIccProfile(const QColorSpace &space)
     return iccProfile;
 }
 
-bool parseTRC(const GenericTagData *trcData, QColorTrc &gamma)
+struct TagEntry {
+    quint32 offset;
+    quint32 size;
+};
+
+bool parseXyzData(const QByteArray &data, const TagEntry &tagEntry, QColorVector &colorVector)
+{
+    if (tagEntry.size < sizeof(XYZTagData)) {
+        qCWarning(lcIcc) << "Undersized XYZ tag";
+        return false;
+    }
+    const XYZTagData *xyz = reinterpret_cast<const XYZTagData *>(data.constData() + tagEntry.offset);
+    if (xyz->type != quint32(Tag::XYZ_)) {
+        qCWarning(lcIcc) << "Bad XYZ content type";
+        return false;
+    }
+    const float x = fromFixedS1516(xyz->fixedX);
+    const float y = fromFixedS1516(xyz->fixedY);
+    const float z = fromFixedS1516(xyz->fixedZ);
+
+    colorVector = QColorVector(x, y, z);
+    return true;
+}
+
+bool parseTRC(const QByteArray &data, const TagEntry &tagEntry, QColorTrc &gamma)
 {
+    const GenericTagData *trcData = reinterpret_cast<const GenericTagData *>(data.constData() + tagEntry.offset);
     if (trcData->type == quint32(Tag::curv)) {
-        const CurvTagData *curv = reinterpret_cast<const CurvTagData *>(trcData);
-        qCDebug(lcIcc) << "curv" << uint(curv->valueCount);
+        const CurvTagData *curv = static_cast<const CurvTagData *>(trcData);
+        if (curv->valueCount > (1 << 16))
+            return false;
+        if (tagEntry.size - 12 < 2 * curv->valueCount)
+            return false;
         if (curv->valueCount == 0) {
             gamma.m_type = QColorTrc::Type::Function;
             gamma.m_fun = QColorTransferFunction(); // Linear
         } else if (curv->valueCount == 1) {
             float g = curv->value[0] * (1.0f / 256.0f);
-            qCDebug(lcIcc) << g;
             gamma.m_type = QColorTrc::Type::Function;
             gamma.m_fun = QColorTransferFunction::fromGamma(g);
         } else {
@@ -445,49 +466,54 @@ bool parseTRC(const GenericTagData *trcData, QColorTrc &gamma)
         return true;
     }
     if (trcData->type == quint32(Tag::para)) {
-        const ParaTagData *para = reinterpret_cast<const ParaTagData *>(trcData);
-        qCDebug(lcIcc) << "para" << uint(para->curveType);
+        if (tagEntry.size < sizeof(ParaTagData))
+            return false;
+        const ParaTagData *para = static_cast<const ParaTagData *>(trcData);
         switch (para->curveType) {
         case 0: {
             float g = fromFixedS1516(para->parameter[0]);
-            qCDebug(lcIcc) << g;
             gamma.m_type = QColorTrc::Type::Function;
             gamma.m_fun = QColorTransferFunction::fromGamma(g);
             break;
         }
         case 1: {
+            if (tagEntry.size < sizeof(ParaTagData) + 2 * 4)
+                return false;
             float g = fromFixedS1516(para->parameter[0]);
             float a = fromFixedS1516(para->parameter[1]);
             float b = fromFixedS1516(para->parameter[2]);
             float d = -b / a;
-            qCDebug(lcIcc) << g << a << b;
             gamma.m_type = QColorTrc::Type::Function;
             gamma.m_fun = QColorTransferFunction(a, b, 0.0f, d, 0.0f, 0.0f, g);
             break;
         }
         case 2: {
+            if (tagEntry.size < sizeof(ParaTagData) + 3 * 4)
+                return false;
             float g = fromFixedS1516(para->parameter[0]);
             float a = fromFixedS1516(para->parameter[1]);
             float b = fromFixedS1516(para->parameter[2]);
             float c = fromFixedS1516(para->parameter[3]);
             float d = -b / a;
-            qCDebug(lcIcc) << g << a << b << c;
             gamma.m_type = QColorTrc::Type::Function;
             gamma.m_fun = QColorTransferFunction(a, b, 0.0f, d, c, c, g);
             break;
         }
         case 3: {
+            if (tagEntry.size < sizeof(ParaTagData) + 4 * 4)
+                return false;
             float g = fromFixedS1516(para->parameter[0]);
             float a = fromFixedS1516(para->parameter[1]);
             float b = fromFixedS1516(para->parameter[2]);
             float c = fromFixedS1516(para->parameter[3]);
             float d = fromFixedS1516(para->parameter[4]);
-            qCDebug(lcIcc) << g << a << b << c << d;
             gamma.m_type = QColorTrc::Type::Function;
             gamma.m_fun = QColorTransferFunction(a, b, c, d, 0.0f, 0.0f, g);
             break;
         }
         case 4: {
+            if (tagEntry.size < sizeof(ParaTagData) + 6 * 4)
+                return false;
             float g = fromFixedS1516(para->parameter[0]);
             float a = fromFixedS1516(para->parameter[1]);
             float b = fromFixedS1516(para->parameter[2]);
@@ -495,7 +521,6 @@ bool parseTRC(const GenericTagData *trcData, QColorTrc &gamma)
             float d = fromFixedS1516(para->parameter[4]);
             float e = fromFixedS1516(para->parameter[5]);
             float f = fromFixedS1516(para->parameter[6]);
-            qCDebug(lcIcc) << g << a << b << c << d << e << f;
             gamma.m_type = QColorTrc::Type::Function;
             gamma.m_fun = QColorTransferFunction(a, b, c, d, e, f, g);
             break;
@@ -529,8 +554,12 @@ bool fromIccProfile(const QByteArray &data, QColorSpace *colorSpace)
     // Read tag index
     const TagTableEntry *tagTable = (const TagTableEntry *)(data.constData() + sizeof(ICCProfileHeader));
     const qsizetype offsetToData = sizeof(ICCProfileHeader) + header->tagCount * sizeof(TagTableEntry);
+    if (offsetToData > data.size()) {
+        qCWarning(lcIcc) << "fromIccProfile: failed index size sanity";
+        return false;
+    }
 
-    QHash<Tag, quint32> tagIndex;
+    QHash<Tag, TagEntry> tagIndex;
     for (uint i = 0; i < header->tagCount; ++i) {
         // Sanity check tag sizes and offsets:
         if (qsizetype(tagTable[i].offset) < offsetToData) {
@@ -542,15 +571,24 @@ bool fromIccProfile(const QByteArray &data, QColorSpace *colorSpace)
             qCWarning(lcIcc) << "fromIccProfile: failed tag offset sanity 2";
             return false;
         }
-        if ((tagTable[i].offset + tagTable[i].size) > header->profileSize) {
+        if (tagTable[i].size < 12) {
+            qCWarning(lcIcc) << "fromIccProfile: failed minimal tag size sanity";
+            return false;
+        }
+        if (tagTable[i].size > header->profileSize - tagTable[i].offset) {
             qCWarning(lcIcc) << "fromIccProfile: failed tag offset + size sanity";
             return false;
         }
+        if (tagTable[i].offset & 0x03) {
+            qCWarning(lcIcc) << "fromIccProfile: invalid tag offset alignment";
+            return false;
+        }
 //        printf("'%4s' %d %d\n", (const char *)&tagTable[i].signature,
 //                                quint32(tagTable[i].offset),
 //                                quint32(tagTable[i].size));
-        tagIndex.insert(Tag(quint32(tagTable[i].signature)), tagTable[i].offset);
+        tagIndex.insert(Tag(quint32(tagTable[i].signature)), { tagTable[i].offset, tagTable[i].size });
     }
+
     // Check the profile is three-component matrix based (what we currently support):
     if (!tagIndex.contains(Tag::rXYZ) || !tagIndex.contains(Tag::gXYZ) || !tagIndex.contains(Tag::bXYZ) ||
         !tagIndex.contains(Tag::rTRC) || !tagIndex.contains(Tag::gTRC) || !tagIndex.contains(Tag::bTRC) ||
@@ -559,23 +597,17 @@ bool fromIccProfile(const QByteArray &data, QColorSpace *colorSpace)
         return false;
     }
 
-    // Parse XYZ tags
-    const XYZTagData *rXyz = (const XYZTagData *)(data.constData() + tagIndex[Tag::rXYZ]);
-    const XYZTagData *gXyz = (const XYZTagData *)(data.constData() + tagIndex[Tag::gXYZ]);
-    const XYZTagData *bXyz = (const XYZTagData *)(data.constData() + tagIndex[Tag::bXYZ]);
-    const XYZTagData *wXyz = (const XYZTagData *)(data.constData() + tagIndex[Tag::wtpt]);
-    if (rXyz->type != quint32(Tag::XYZ_) || gXyz->type != quint32(Tag::XYZ_) ||
-        wXyz->type != quint32(Tag::XYZ_) || wXyz->type != quint32(Tag::XYZ_)) {
-        qCWarning(lcIcc) << "fromIccProfile: Bad XYZ data type";
-        return false;
-    }
     QColorSpacePrivate *colorspaceDPtr = QColorSpacePrivate::getWritable(*colorSpace);
 
-    colorspaceDPtr->toXyz.r = fromXyzData(rXyz);
-    colorspaceDPtr->toXyz.g = fromXyzData(gXyz);
-    colorspaceDPtr->toXyz.b = fromXyzData(bXyz);
-    QColorVector whitePoint = fromXyzData(wXyz);
-    colorspaceDPtr->whitePoint = whitePoint;
+    // Parse XYZ tags
+    if (!parseXyzData(data, tagIndex[Tag::rXYZ], colorspaceDPtr->toXyz.r))
+        return false;
+    if (!parseXyzData(data, tagIndex[Tag::gXYZ], colorspaceDPtr->toXyz.g))
+        return false;
+    if (!parseXyzData(data, tagIndex[Tag::bXYZ], colorspaceDPtr->toXyz.b))
+        return false;
+    if (!parseXyzData(data, tagIndex[Tag::wtpt], colorspaceDPtr->whitePoint))
+        return false;
 
     colorspaceDPtr->gamut = QColorSpace::Gamut::Custom;
     if (colorspaceDPtr->toXyz == QColorMatrix::toXyzFromSRgb()) {
@@ -600,29 +632,35 @@ bool fromIccProfile(const QByteArray &data, QColorSpace *colorSpace)
         colorspaceDPtr->setToXyzMatrix();
 
     // Parse TRC tags
-    const GenericTagData *rTrc;
-    const GenericTagData *gTrc;
-    const GenericTagData *bTrc;
+    TagEntry rTrc;
+    TagEntry gTrc;
+    TagEntry bTrc;
     if (tagIndex.contains(Tag::aarg) && tagIndex.contains(Tag::aagg) && tagIndex.contains(Tag::aabg)) {
         // Apple extension for parametric version of TRCs in ICCv2:
-        rTrc = (const GenericTagData *)(data.constData() + tagIndex[Tag::aarg]);
-        gTrc = (const GenericTagData *)(data.constData() + tagIndex[Tag::aagg]);
-        bTrc = (const GenericTagData *)(data.constData() + tagIndex[Tag::aabg]);
+        rTrc = tagIndex[Tag::aarg];
+        gTrc = tagIndex[Tag::aagg];
+        bTrc = tagIndex[Tag::aabg];
     } else {
-        rTrc = (const GenericTagData *)(data.constData() + tagIndex[Tag::rTRC]);
-        gTrc = (const GenericTagData *)(data.constData() + tagIndex[Tag::gTRC]);
-        bTrc = (const GenericTagData *)(data.constData() + tagIndex[Tag::bTRC]);
+        rTrc = tagIndex[Tag::rTRC];
+        gTrc = tagIndex[Tag::gTRC];
+        bTrc = tagIndex[Tag::bTRC];
     }
 
     QColorTrc rCurve;
     QColorTrc gCurve;
     QColorTrc bCurve;
-    if (!parseTRC(rTrc, rCurve))
+    if (!parseTRC(data, rTrc, rCurve)) {
+        qCWarning(lcIcc) << "fromIccProfile: Invalid rTRC";
         return false;
-    if (!parseTRC(gTrc, gCurve))
+    }
+    if (!parseTRC(data, gTrc, gCurve)) {
+        qCWarning(lcIcc) << "fromIccProfile: Invalid gTRC";
         return false;
-    if (!parseTRC(bTrc, bCurve))
+    }
+    if (!parseTRC(data, bTrc, bCurve)) {
+        qCWarning(lcIcc) << "fromIccProfile: Invalid bTRC";
         return false;
+    }
     if (rCurve == gCurve && gCurve == bCurve && rCurve.m_type == QColorTrc::Type::Function) {
         if (rCurve.m_fun.isLinear()) {
             qCDebug(lcIcc) << "fromIccProfile: Linear gamma detected";