From e750fa8509d350cbbcddc81174516745f6ffe9f3 Mon Sep 17 00:00:00 2001
From: Allan Sandfeld Jensen <allan.jensen@qt.io>
Date: Sun, 27 Sep 2020 10:27:10 +0200
Subject: Do not assert on invalid ICC curv table

Reject invalid table data, do not assert it.

Pick-to: 5.15
Change-Id: I1fdda4c735bb97bb93ac8f8654ac4cc5fada5389
Reviewed-by: Eirik Aavitsland <eirik.aavitsland@qt.io>
---
 src/gui/painting/qcolortransfertable_p.h | 47 +++++++++++++++++++++-----------
 src/gui/painting/qicc.cpp                |  5 +++-
 2 files changed, 35 insertions(+), 17 deletions(-)

(limited to 'src/gui/painting')

diff --git a/src/gui/painting/qcolortransfertable_p.h b/src/gui/painting/qcolortransfertable_p.h
index c945b55576..b518df13b9 100644
--- a/src/gui/painting/qcolortransfertable_p.h
+++ b/src/gui/painting/qcolortransfertable_p.h
@@ -68,42 +68,56 @@ public:
     { }
     QColorTransferTable(uint32_t size, const QList<uint8_t> &table) noexcept
         : m_tableSize(size), m_table8(table)
-    { }
+    {
+        Q_ASSERT(size <= table.count());
+    }
     QColorTransferTable(uint32_t size, const QList<uint16_t> &table) noexcept
         : m_tableSize(size), m_table16(table)
-    { }
+    {
+        Q_ASSERT(size <= table.count());
+    }
 
-    bool isValid() const
+    bool isEmpty() const
     {
+        return m_tableSize == 0;
+    }
+
+    bool checkValidity() const
+    {
+        if (isEmpty())
+            return true;
+        // Only one table can be set
+        if (!m_table8.isEmpty() && !m_table16.isEmpty())
+            return false;
+        // At least 2 elements
         if (m_tableSize < 2)
             return false;
-
-#if !defined(QT_NO_DEBUG)
         // The table must describe an injective curve:
         if (!m_table8.isEmpty()) {
             uint8_t val = 0;
             for (uint i = 0; i < m_tableSize; ++i) {
-                Q_ASSERT(m_table8[i] >= val);
+                if (m_table8[i] < val)
+                    return false;
                 val = m_table8[i];
             }
         }
         if (!m_table16.isEmpty()) {
             uint16_t val = 0;
             for (uint i = 0; i < m_tableSize; ++i) {
-                Q_ASSERT(m_table16[i] >= val);
+                if (m_table16[i] < val)
+                    return false;
                 val = m_table16[i];
             }
         }
-#endif
-        return !m_table8.isEmpty() || !m_table16.isEmpty();
+        return true;
     }
 
     float apply(float x) const
     {
         x = std::min(std::max(x, 0.0f), 1.0f);
         x *= m_tableSize - 1;
-        uint32_t lo = (int)std::floor(x);
-        uint32_t hi = std::min(lo + 1, m_tableSize);
+        uint32_t lo = static_cast<uint32_t>(std::floor(x));
+        uint32_t hi = std::min(lo + 1, m_tableSize - 1);
         float frac = x - lo;
         if (!m_table16.isEmpty())
             return (m_table16[lo] * (1.0f - frac) + m_table16[hi] * frac) * (1.0f/65535.0f);
@@ -122,7 +136,7 @@ public:
             return 1.0f;
         if (!m_table16.isEmpty()) {
             float v = x * 65535.0f;
-            uint i = std::floor(resultLargerThan * (m_tableSize - 1)) + 1;
+            uint32_t i = std::floor(resultLargerThan * (m_tableSize - 1)) + 1;
             for ( ; i < m_tableSize; ++i) {
                 if (m_table16[i] > v)
                     break;
@@ -138,7 +152,7 @@ public:
         }
         if (!m_table8.isEmpty()) {
             float v = x * 255.0f;
-            uint i = std::floor(resultLargerThan * (m_tableSize - 1)) + 1;
+            uint32_t i = std::floor(resultLargerThan * (m_tableSize - 1)) + 1;
             for ( ; i < m_tableSize; ++i) {
                 if (m_table8[i] > v)
                     break;
@@ -156,8 +170,9 @@ public:
 
     bool asColorTransferFunction(QColorTransferFunction *transferFn)
     {
-        Q_ASSERT(isValid());
         Q_ASSERT(transferFn);
+        if (m_tableSize < 2)
+            return false;
         if (!m_table8.isEmpty() && (m_table8[0] != 0 || m_table8[m_tableSize - 1] != 255))
             return false;
         if (!m_table16.isEmpty() && (m_table16[0] != 0 || m_table16[m_tableSize - 1] != 65535))
@@ -219,13 +234,13 @@ inline bool operator!=(const QColorTransferTable &t1, const QColorTransferTable
     if (t1.m_table16.isEmpty() != t2.m_table16.isEmpty())
         return true;
     if (!t1.m_table8.isEmpty()) {
-        for (quint32 i = 0; i < t1.m_tableSize; ++i) {
+        for (uint32_t i = 0; i < t1.m_tableSize; ++i) {
             if (t1.m_table8[i] != t2.m_table8[i])
                 return true;
         }
     }
     if (!t1.m_table16.isEmpty()) {
-        for (quint32 i = 0; i < t1.m_tableSize; ++i) {
+        for (uint32_t i = 0; i < t1.m_tableSize; ++i) {
             if (t1.m_table16[i] != t2.m_table16[i])
                 return true;
         }
diff --git a/src/gui/painting/qicc.cpp b/src/gui/painting/qicc.cpp
index 7cf5e36073..746ef43692 100644
--- a/src/gui/painting/qicc.cpp
+++ b/src/gui/painting/qicc.cpp
@@ -489,7 +489,10 @@ bool parseTRC(const QByteArray &data, const TagEntry &tagEntry, QColorTrc &gamma
             qFromBigEndian<quint16>(data.constData() + offset, curv.valueCount, tabl.data());
             QColorTransferTable table = QColorTransferTable(curv.valueCount, std::move(tabl));
             QColorTransferFunction curve;
-            if (!table.asColorTransferFunction(&curve)) {
+            if (!table.checkValidity()) {
+                qCWarning(lcIcc) << "Invalid curv table";
+                return false;
+            } else if (!table.asColorTransferFunction(&curve)) {
                 gamma.m_type = QColorTrc::Type::Table;
                 gamma.m_table = table;
             } else {
-- 
cgit v1.2.3