QIcc: fix alignment concerns in ICC profile parsing

Updated QIcc::fromIccProfile() and friends to not rely on
QByteArray pointer alignment. Used qFromUnaligned() instead

Task-number: QTBUG-84267
Pick-to: 5.15
Change-Id: I69ef7e011707bec27cd84693e7f0e92d79a577d1
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
This commit is contained in:
Andrei Golubev 2020-09-01 15:48:00 +02:00
parent ecf4f8fc64
commit d423fe9851
2 changed files with 104 additions and 87 deletions

View File

@ -666,9 +666,6 @@ QByteArray QColorSpace::iccProfile() const
If the ICC profile is not supported an invalid QColorSpace is returned If the ICC profile is not supported an invalid QColorSpace is returned
where you can still read the original ICC profile using iccProfile(). where you can still read the original ICC profile using iccProfile().
\note If the QByteArray data is created from external sources it should be
at least 4 byte aligned.
\sa iccProfile() \sa iccProfile()
*/ */
QColorSpace QColorSpace::fromIccProfile(const QByteArray &iccProfile) QColorSpace QColorSpace::fromIccProfile(const QByteArray &iccProfile)

View File

@ -1,6 +1,6 @@
/**************************************************************************** /****************************************************************************
** **
** Copyright (C) 2016 The Qt Company Ltd. ** Copyright (C) 2020 The Qt Company Ltd.
** Contact: https://www.qt.io/licensing/ ** Contact: https://www.qt.io/licensing/
** **
** This file is part of the QtGui module of the Qt Toolkit. ** This file is part of the QtGui module of the Qt Toolkit.
@ -49,6 +49,8 @@
#include "qcolorspace_p.h" #include "qcolorspace_p.h"
#include "qcolortrc_p.h" #include "qcolortrc_p.h"
#include <array>
QT_BEGIN_NAMESPACE QT_BEGIN_NAMESPACE
Q_LOGGING_CATEGORY(lcIcc, "qt.gui.icc") Q_LOGGING_CATEGORY(lcIcc, "qt.gui.icc")
@ -448,14 +450,14 @@ bool parseXyzData(const QByteArray &data, const TagEntry &tagEntry, QColorVector
qCWarning(lcIcc) << "Undersized XYZ tag"; qCWarning(lcIcc) << "Undersized XYZ tag";
return false; return false;
} }
const XYZTagData *xyz = reinterpret_cast<const XYZTagData *>(data.constData() + tagEntry.offset); const XYZTagData xyz = qFromUnaligned<XYZTagData>(data.constData() + tagEntry.offset);
if (xyz->type != quint32(Tag::XYZ_)) { if (xyz.type != quint32(Tag::XYZ_)) {
qCWarning(lcIcc) << "Bad XYZ content type"; qCWarning(lcIcc) << "Bad XYZ content type";
return false; return false;
} }
const float x = fromFixedS1516(xyz->fixedX); const float x = fromFixedS1516(xyz.fixedX);
const float y = fromFixedS1516(xyz->fixedY); const float y = fromFixedS1516(xyz.fixedY);
const float z = fromFixedS1516(xyz->fixedZ); const float z = fromFixedS1516(xyz.fixedZ);
colorVector = QColorVector(x, y, z); colorVector = QColorVector(x, y, z);
return true; return true;
@ -463,26 +465,29 @@ bool parseXyzData(const QByteArray &data, const TagEntry &tagEntry, QColorVector
bool parseTRC(const QByteArray &data, const TagEntry &tagEntry, QColorTrc &gamma) bool parseTRC(const QByteArray &data, const TagEntry &tagEntry, QColorTrc &gamma)
{ {
const GenericTagData *trcData = reinterpret_cast<const GenericTagData *>(data.constData() + tagEntry.offset); const GenericTagData trcData = qFromUnaligned<GenericTagData>(data.constData()
if (trcData->type == quint32(Tag::curv)) { + tagEntry.offset);
const CurvTagData *curv = static_cast<const CurvTagData *>(trcData); if (trcData.type == quint32(Tag::curv)) {
if (curv->valueCount > (1 << 16)) const CurvTagData curv = qFromUnaligned<CurvTagData>(data.constData() + tagEntry.offset);
if (curv.valueCount > (1 << 16))
return false; return false;
if (tagEntry.size - 12 < 2 * curv->valueCount) if (tagEntry.size - 12 < 2 * curv.valueCount)
return false; return false;
if (curv->valueCount == 0) { if (curv.valueCount == 0) {
gamma.m_type = QColorTrc::Type::Function; gamma.m_type = QColorTrc::Type::Function;
gamma.m_fun = QColorTransferFunction(); // Linear gamma.m_fun = QColorTransferFunction(); // Linear
} else if (curv->valueCount == 1) { } else if (curv.valueCount == 1) {
float g = curv->value[0] * (1.0f / 256.0f); float g = curv.value[0] * (1.0f / 256.0f);
gamma.m_type = QColorTrc::Type::Function; gamma.m_type = QColorTrc::Type::Function;
gamma.m_fun = QColorTransferFunction::fromGamma(g); gamma.m_fun = QColorTransferFunction::fromGamma(g);
} else { } else {
QList<quint16> tabl; QList<quint16> tabl;
tabl.resize(curv->valueCount); tabl.resize(curv.valueCount);
for (uint i = 0; i < curv->valueCount; ++i) static_assert(sizeof(GenericTagData) == 2 * sizeof(quint32_be),
tabl[i] = curv->value[i]; "GenericTagData has padding. The following code is a subject to UB.");
QColorTransferTable table = QColorTransferTable(curv->valueCount, std::move(tabl)); const auto offset = tagEntry.offset + sizeof(GenericTagData) + sizeof(quint32_be);
qFromBigEndian<quint16>(data.constData() + offset, curv.valueCount, tabl.data());
QColorTransferTable table = QColorTransferTable(curv.valueCount, std::move(tabl));
QColorTransferFunction curve; QColorTransferFunction curve;
if (!table.asColorTransferFunction(&curve)) { if (!table.asColorTransferFunction(&curve)) {
gamma.m_type = QColorTrc::Type::Table; gamma.m_type = QColorTrc::Type::Table;
@ -495,13 +500,18 @@ bool parseTRC(const QByteArray &data, const TagEntry &tagEntry, QColorTrc &gamma
} }
return true; return true;
} }
if (trcData->type == quint32(Tag::para)) { if (trcData.type == quint32(Tag::para)) {
if (tagEntry.size < sizeof(ParaTagData)) if (tagEntry.size < sizeof(ParaTagData))
return false; return false;
const ParaTagData *para = static_cast<const ParaTagData *>(trcData); static_assert(sizeof(GenericTagData) == 2 * sizeof(quint32_be),
switch (para->curveType) { "GenericTagData has padding. The following code is a subject to UB.");
const ParaTagData para = qFromUnaligned<ParaTagData>(data.constData() + tagEntry.offset);
// re-read first parameter for consistency:
const auto parametersOffset = tagEntry.offset + sizeof(GenericTagData)
+ 2 * sizeof(quint16_be);
switch (para.curveType) {
case 0: { case 0: {
float g = fromFixedS1516(para->parameter[0]); float g = fromFixedS1516(para.parameter[0]);
gamma.m_type = QColorTrc::Type::Function; gamma.m_type = QColorTrc::Type::Function;
gamma.m_fun = QColorTransferFunction::fromGamma(g); gamma.m_fun = QColorTransferFunction::fromGamma(g);
break; break;
@ -509,9 +519,11 @@ bool parseTRC(const QByteArray &data, const TagEntry &tagEntry, QColorTrc &gamma
case 1: { case 1: {
if (tagEntry.size < sizeof(ParaTagData) + 2 * 4) if (tagEntry.size < sizeof(ParaTagData) + 2 * 4)
return false; return false;
float g = fromFixedS1516(para->parameter[0]); std::array<quint32_be, 3> parameters =
float a = fromFixedS1516(para->parameter[1]); qFromUnaligned<decltype(parameters)>(data.constData() + parametersOffset);
float b = fromFixedS1516(para->parameter[2]); float g = fromFixedS1516(parameters[0]);
float a = fromFixedS1516(parameters[1]);
float b = fromFixedS1516(parameters[2]);
float d = -b / a; float d = -b / a;
gamma.m_type = QColorTrc::Type::Function; gamma.m_type = QColorTrc::Type::Function;
gamma.m_fun = QColorTransferFunction(a, b, 0.0f, d, 0.0f, 0.0f, g); gamma.m_fun = QColorTransferFunction(a, b, 0.0f, d, 0.0f, 0.0f, g);
@ -520,10 +532,12 @@ bool parseTRC(const QByteArray &data, const TagEntry &tagEntry, QColorTrc &gamma
case 2: { case 2: {
if (tagEntry.size < sizeof(ParaTagData) + 3 * 4) if (tagEntry.size < sizeof(ParaTagData) + 3 * 4)
return false; return false;
float g = fromFixedS1516(para->parameter[0]); std::array<quint32_be, 4> parameters =
float a = fromFixedS1516(para->parameter[1]); qFromUnaligned<decltype(parameters)>(data.constData() + parametersOffset);
float b = fromFixedS1516(para->parameter[2]); float g = fromFixedS1516(parameters[0]);
float c = fromFixedS1516(para->parameter[3]); float a = fromFixedS1516(parameters[1]);
float b = fromFixedS1516(parameters[2]);
float c = fromFixedS1516(parameters[3]);
float d = -b / a; float d = -b / a;
gamma.m_type = QColorTrc::Type::Function; gamma.m_type = QColorTrc::Type::Function;
gamma.m_fun = QColorTransferFunction(a, b, 0.0f, d, c, c, g); gamma.m_fun = QColorTransferFunction(a, b, 0.0f, d, c, c, g);
@ -532,11 +546,13 @@ bool parseTRC(const QByteArray &data, const TagEntry &tagEntry, QColorTrc &gamma
case 3: { case 3: {
if (tagEntry.size < sizeof(ParaTagData) + 4 * 4) if (tagEntry.size < sizeof(ParaTagData) + 4 * 4)
return false; return false;
float g = fromFixedS1516(para->parameter[0]); std::array<quint32_be, 5> parameters =
float a = fromFixedS1516(para->parameter[1]); qFromUnaligned<decltype(parameters)>(data.constData() + parametersOffset);
float b = fromFixedS1516(para->parameter[2]); float g = fromFixedS1516(parameters[0]);
float c = fromFixedS1516(para->parameter[3]); float a = fromFixedS1516(parameters[1]);
float d = fromFixedS1516(para->parameter[4]); float b = fromFixedS1516(parameters[2]);
float c = fromFixedS1516(parameters[3]);
float d = fromFixedS1516(parameters[4]);
gamma.m_type = QColorTrc::Type::Function; gamma.m_type = QColorTrc::Type::Function;
gamma.m_fun = QColorTransferFunction(a, b, c, d, 0.0f, 0.0f, g); gamma.m_fun = QColorTransferFunction(a, b, c, d, 0.0f, 0.0f, g);
break; break;
@ -544,19 +560,21 @@ bool parseTRC(const QByteArray &data, const TagEntry &tagEntry, QColorTrc &gamma
case 4: { case 4: {
if (tagEntry.size < sizeof(ParaTagData) + 6 * 4) if (tagEntry.size < sizeof(ParaTagData) + 6 * 4)
return false; return false;
float g = fromFixedS1516(para->parameter[0]); std::array<quint32_be, 7> parameters =
float a = fromFixedS1516(para->parameter[1]); qFromUnaligned<decltype(parameters)>(data.constData() + parametersOffset);
float b = fromFixedS1516(para->parameter[2]); float g = fromFixedS1516(parameters[0]);
float c = fromFixedS1516(para->parameter[3]); float a = fromFixedS1516(parameters[1]);
float d = fromFixedS1516(para->parameter[4]); float b = fromFixedS1516(parameters[2]);
float e = fromFixedS1516(para->parameter[5]); float c = fromFixedS1516(parameters[3]);
float f = fromFixedS1516(para->parameter[6]); float d = fromFixedS1516(parameters[4]);
float e = fromFixedS1516(parameters[5]);
float f = fromFixedS1516(parameters[6]);
gamma.m_type = QColorTrc::Type::Function; gamma.m_type = QColorTrc::Type::Function;
gamma.m_fun = QColorTransferFunction(a, b, c, d, e, f, g); gamma.m_fun = QColorTransferFunction(a, b, c, d, e, f, g);
break; break;
} }
default: default:
qCWarning(lcIcc) << "Unknown para type" << uint(para->curveType); qCWarning(lcIcc) << "Unknown para type" << uint(para.curveType);
return false; return false;
} }
return true; return true;
@ -567,73 +585,70 @@ bool parseTRC(const QByteArray &data, const TagEntry &tagEntry, QColorTrc &gamma
bool parseDesc(const QByteArray &data, const TagEntry &tagEntry, QString &descName) bool parseDesc(const QByteArray &data, const TagEntry &tagEntry, QString &descName)
{ {
const GenericTagData *tag = (const GenericTagData *)(data.constData() + tagEntry.offset); const GenericTagData tag = qFromUnaligned<GenericTagData>(data.constData() + tagEntry.offset);
// Either 'desc' (ICCv2) or 'mluc' (ICCv4) // Either 'desc' (ICCv2) or 'mluc' (ICCv4)
if (tag->type == quint32(Tag::desc)) { if (tag.type == quint32(Tag::desc)) {
if (tagEntry.size < sizeof(DescTagData)) if (tagEntry.size < sizeof(DescTagData))
return false; return false;
const DescTagData *desc = (const DescTagData *)(data.constData() + tagEntry.offset); const DescTagData desc = qFromUnaligned<DescTagData>(data.constData() + tagEntry.offset);
const quint32 len = desc->asciiDescriptionLength; const quint32 len = desc.asciiDescriptionLength;
if (len < 1) if (len < 1)
return false; return false;
if (tagEntry.size - 12 < len) if (tagEntry.size - 12 < len)
return false; return false;
if (desc->asciiDescription[len - 1] != '\0') static_assert(sizeof(GenericTagData) == 2 * sizeof(quint32_be),
"GenericTagData has padding. The following code is a subject to UB.");
const char *asciiDescription = data.constData() + tagEntry.offset + sizeof(GenericTagData)
+ sizeof(quint32_be);
if (asciiDescription[len - 1] != '\0')
return false; return false;
descName = QString::fromLatin1(desc->asciiDescription, len - 1); descName = QString::fromLatin1(asciiDescription, len - 1);
return true; return true;
} }
if (tag->type != quint32(Tag::mluc)) if (tag.type != quint32(Tag::mluc))
return false; return false;
if (tagEntry.size < sizeof(MlucTagData)) if (tagEntry.size < sizeof(MlucTagData))
return false; return false;
const MlucTagData *mluc = (const MlucTagData *)(data.constData() + tagEntry.offset); const MlucTagData mluc = qFromUnaligned<MlucTagData>(data.constData() + tagEntry.offset);
if (mluc->recordCount < 1) if (mluc.recordCount < 1)
return false; return false;
if (mluc->recordSize < 12) if (mluc.recordSize < 12)
return false; return false;
// We just use the primary record regardless of language or country. // We just use the primary record regardless of language or country.
const quint32 stringOffset = mluc->records[0].offset; const quint32 stringOffset = mluc.records[0].offset;
const quint32 stringSize = mluc->records[0].size; const quint32 stringSize = mluc.records[0].size;
if (tagEntry.size < stringOffset || tagEntry.size - stringOffset < stringSize ) if (tagEntry.size < stringOffset || tagEntry.size - stringOffset < stringSize )
return false; return false;
if ((stringSize | stringOffset) & 1) if ((stringSize | stringOffset) & 1)
return false; return false;
quint32 stringLen = stringSize / 2; quint32 stringLen = stringSize / 2;
const ushort *unicodeString = (const ushort *)(data.constData() + tagEntry.offset + stringOffset);
// The given length shouldn't include 0-termination, but might.
if (stringLen > 1 && unicodeString[stringLen - 1] == 0)
--stringLen;
QVarLengthArray<char16_t> utf16hostendian(stringLen); QVarLengthArray<char16_t> utf16hostendian(stringLen);
qFromBigEndian<char16_t>(unicodeString, stringLen, utf16hostendian.data()); qFromBigEndian<char16_t>(data.constData() + tagEntry.offset + stringOffset, stringLen,
utf16hostendian.data());
// The given length shouldn't include 0-termination, but might.
if (stringLen > 1 && utf16hostendian[stringLen - 1] == 0)
--stringLen;
descName = QString::fromUtf16(utf16hostendian.data(), stringLen); descName = QString::fromUtf16(utf16hostendian.data(), stringLen);
return true; return true;
} }
bool fromIccProfile(const QByteArray &data, QColorSpace *colorSpace) bool fromIccProfile(const QByteArray &data, QColorSpace *colorSpace)
{ {
Q_ASSERT((reinterpret_cast<qintptr>(data.constData()) & 0x3) == 0);
if (reinterpret_cast<qintptr>(data.constData()) & 0x3) {
qCWarning(lcIcc) << "fromIccProfile: Unaligned profile data";
return false;
}
if (data.size() < qsizetype(sizeof(ICCProfileHeader))) { if (data.size() < qsizetype(sizeof(ICCProfileHeader))) {
qCWarning(lcIcc) << "fromIccProfile: failed size sanity 1"; qCWarning(lcIcc) << "fromIccProfile: failed size sanity 1";
return false; return false;
} }
const ICCProfileHeader *header = (const ICCProfileHeader *)data.constData(); const ICCProfileHeader header = qFromUnaligned<ICCProfileHeader>(data.constData());
if (!isValidIccProfile(*header)) if (!isValidIccProfile(header))
return false; // if failed we already printing a warning return false; // if failed we already printing a warning
if (qsizetype(header->profileSize) > data.size()) { if (qsizetype(header.profileSize) > data.size()) {
qCWarning(lcIcc) << "fromIccProfile: failed size sanity 2"; qCWarning(lcIcc) << "fromIccProfile: failed size sanity 2";
return false; return false;
} }
// Read tag index const qsizetype offsetToData = sizeof(ICCProfileHeader) + header.tagCount * sizeof(TagTableEntry);
const TagTableEntry *tagTable = (const TagTableEntry *)(data.constData() + sizeof(ICCProfileHeader));
const qsizetype offsetToData = sizeof(ICCProfileHeader) + header->tagCount * sizeof(TagTableEntry);
Q_ASSERT(offsetToData > 0); Q_ASSERT(offsetToData > 0);
if (offsetToData > data.size()) { if (offsetToData > data.size()) {
qCWarning(lcIcc) << "fromIccProfile: failed index size sanity"; qCWarning(lcIcc) << "fromIccProfile: failed index size sanity";
@ -641,37 +656,42 @@ bool fromIccProfile(const QByteArray &data, QColorSpace *colorSpace)
} }
QHash<Tag, TagEntry> tagIndex; QHash<Tag, TagEntry> tagIndex;
for (uint i = 0; i < header->tagCount; ++i) { for (uint i = 0; i < header.tagCount; ++i) {
// Read tag index
const qsizetype tableOffset = sizeof(ICCProfileHeader) + i * sizeof(TagTableEntry);
const TagTableEntry tagTable = qFromUnaligned<TagTableEntry>(data.constData()
+ tableOffset);
// Sanity check tag sizes and offsets: // Sanity check tag sizes and offsets:
if (qsizetype(tagTable[i].offset) < offsetToData) { if (qsizetype(tagTable.offset) < offsetToData) {
qCWarning(lcIcc) << "fromIccProfile: failed tag offset sanity 1"; qCWarning(lcIcc) << "fromIccProfile: failed tag offset sanity 1";
return false; return false;
} }
// Checked separately from (+ size) to handle overflow. // Checked separately from (+ size) to handle overflow.
if (tagTable[i].offset > header->profileSize) { if (tagTable.offset > header.profileSize) {
qCWarning(lcIcc) << "fromIccProfile: failed tag offset sanity 2"; qCWarning(lcIcc) << "fromIccProfile: failed tag offset sanity 2";
return false; return false;
} }
if (tagTable[i].size < 12) { if (tagTable.size < 12) {
qCWarning(lcIcc) << "fromIccProfile: failed minimal tag size sanity"; qCWarning(lcIcc) << "fromIccProfile: failed minimal tag size sanity";
return false; return false;
} }
if (tagTable[i].size > header->profileSize - tagTable[i].offset) { if (tagTable.size > header.profileSize - tagTable.offset) {
qCWarning(lcIcc) << "fromIccProfile: failed tag offset + size sanity"; qCWarning(lcIcc) << "fromIccProfile: failed tag offset + size sanity";
return false; return false;
} }
if (tagTable[i].offset & 0x03) { if (tagTable.offset & 0x03) {
qCWarning(lcIcc) << "fromIccProfile: invalid tag offset alignment"; qCWarning(lcIcc) << "fromIccProfile: invalid tag offset alignment";
return false; return false;
} }
// printf("'%4s' %d %d\n", (const char *)&tagTable[i].signature, // printf("'%4s' %d %d\n", (const char *)&tagTable.signature,
// quint32(tagTable[i].offset), // quint32(tagTable.offset),
// quint32(tagTable[i].size)); // quint32(tagTable.size));
tagIndex.insert(Tag(quint32(tagTable[i].signature)), { tagTable[i].offset, tagTable[i].size }); tagIndex.insert(Tag(quint32(tagTable.signature)), { tagTable.offset, tagTable.size });
} }
// Check the profile is three-component matrix based (what we currently support): // Check the profile is three-component matrix based (what we currently support):
if (header->inputColorSpace == uint(ColorSpaceType::Rgb)) { if (header.inputColorSpace == uint(ColorSpaceType::Rgb)) {
if (!tagIndex.contains(Tag::rXYZ) || !tagIndex.contains(Tag::gXYZ) || !tagIndex.contains(Tag::bXYZ) || 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) || !tagIndex.contains(Tag::rTRC) || !tagIndex.contains(Tag::gTRC) || !tagIndex.contains(Tag::bTRC) ||
!tagIndex.contains(Tag::wtpt)) { !tagIndex.contains(Tag::wtpt)) {
@ -679,7 +699,7 @@ bool fromIccProfile(const QByteArray &data, QColorSpace *colorSpace)
return false; return false;
} }
} else { } else {
Q_ASSERT(header->inputColorSpace == uint(ColorSpaceType::Gray)); Q_ASSERT(header.inputColorSpace == uint(ColorSpaceType::Gray));
if (!tagIndex.contains(Tag::kTRC) || !tagIndex.contains(Tag::wtpt)) { if (!tagIndex.contains(Tag::kTRC) || !tagIndex.contains(Tag::wtpt)) {
qCWarning(lcIcc) << "fromIccProfile: Invalid ICC profile - not valid gray scale based"; qCWarning(lcIcc) << "fromIccProfile: Invalid ICC profile - not valid gray scale based";
return false; return false;
@ -688,7 +708,7 @@ bool fromIccProfile(const QByteArray &data, QColorSpace *colorSpace)
QColorSpacePrivate *colorspaceDPtr = QColorSpacePrivate::getWritable(*colorSpace); QColorSpacePrivate *colorspaceDPtr = QColorSpacePrivate::getWritable(*colorSpace);
if (header->inputColorSpace == uint(ColorSpaceType::Rgb)) { if (header.inputColorSpace == uint(ColorSpaceType::Rgb)) {
// Parse XYZ tags // Parse XYZ tags
if (!parseXyzData(data, tagIndex[Tag::rXYZ], colorspaceDPtr->toXyz.r)) if (!parseXyzData(data, tagIndex[Tag::rXYZ], colorspaceDPtr->toXyz.r))
return false; return false;
@ -748,7 +768,7 @@ bool fromIccProfile(const QByteArray &data, QColorSpace *colorSpace)
TagEntry rTrc; TagEntry rTrc;
TagEntry gTrc; TagEntry gTrc;
TagEntry bTrc; TagEntry bTrc;
if (header->inputColorSpace == uint(ColorSpaceType::Gray)) { if (header.inputColorSpace == uint(ColorSpaceType::Gray)) {
rTrc = tagIndex[Tag::kTRC]; rTrc = tagIndex[Tag::kTRC];
gTrc = tagIndex[Tag::kTRC]; gTrc = tagIndex[Tag::kTRC];
bTrc = tagIndex[Tag::kTRC]; bTrc = tagIndex[Tag::kTRC];