Skip to content

Commit

Permalink
Do stricter error checking when parsing path nodes
Browse files Browse the repository at this point in the history 
The SVG spec mandates that path parsing should terminate on the first
error encountered, and an error be reported. To improve the handling
of corrupt files, implement such error handling, and also limit the
number of QPainterPath elements to a reasonable range.

Fixes: QTBUG-96044
Pick-to: 6.2 5.15 5.12
Change-Id: Ic5e65d6b658516d6f1317c72de365c8c7ad81891
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
Reviewed-by: Robert Löhning <robert.loehning@qt.io>
  • Loading branch information
@aavit
aavit committed Oct 27, 2021
1 parent 14b3677 commit 36cfd9e
Showing 1 changed file with 25 additions and 34 deletions.
59 changes: 25 additions & 34 deletions src/svg/qsvghandler.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1595,14 +1595,16 @@ static void pathArc(QPainterPath &path,

static bool parsePathDataFast(QStringView dataStr, QPainterPath &path)
{
const int maxElementCount = 0x7fff; // Assume file corruption if more path elements than this
qreal x0 = 0, y0 = 0; // starting point
qreal x = 0, y = 0; // current point
char lastMode = 0;
QPointF ctrlPt;
const QChar *str = dataStr.constData();
const QChar *end = str + dataStr.size();

while (str != end) {
bool ok = true;
while (ok && str != end) {
while (str->isSpace() && (str + 1) != end)
++str;
QChar pathElem = *str;
Expand All @@ -1619,14 +1621,13 @@ static bool parsePathDataFast(QStringView dataStr, QPainterPath &path)
arg.append(0);//dummy
const qreal *num = arg.constData();
int count = arg.count();
while (count > 0) {
while (ok && count > 0) {
qreal offsetX = x; // correction offsets
qreal offsetY = y; // for relative commands
switch (pathElem.unicode()) {
case 'm': {
if (count < 2) {
num++;
count--;
ok = false;
break;
}
x = x0 = num[0] + offsetX;
Expand All @@ -1643,8 +1644,7 @@ static bool parsePathDataFast(QStringView dataStr, QPainterPath &path)
break;
case 'M': {
if (count < 2) {
num++;
count--;
ok = false;
break;
}
x = x0 = num[0];
Expand All @@ -1670,8 +1670,7 @@ static bool parsePathDataFast(QStringView dataStr, QPainterPath &path)
break;
case 'l': {
if (count < 2) {
num++;
count--;
ok = false;
break;
}
x = num[0] + offsetX;
Expand All @@ -1684,8 +1683,7 @@ static bool parsePathDataFast(QStringView dataStr, QPainterPath &path)
break;
case 'L': {
if (count < 2) {
num++;
count--;
ok = false;
break;
}
x = num[0];
Expand Down Expand Up @@ -1725,8 +1723,7 @@ static bool parsePathDataFast(QStringView dataStr, QPainterPath &path)
break;
case 'c': {
if (count < 6) {
num += count;
count = 0;
ok = false;
break;
}
QPointF c1(num[0] + offsetX, num[1] + offsetY);
Expand All @@ -1742,8 +1739,7 @@ static bool parsePathDataFast(QStringView dataStr, QPainterPath &path)
}
case 'C': {
if (count < 6) {
num += count;
count = 0;
ok = false;
break;
}
QPointF c1(num[0], num[1]);
Expand All @@ -1759,8 +1755,7 @@ static bool parsePathDataFast(QStringView dataStr, QPainterPath &path)
}
case 's': {
if (count < 4) {
num += count;
count = 0;
ok = false;
break;
}
QPointF c1;
Expand All @@ -1781,8 +1776,7 @@ static bool parsePathDataFast(QStringView dataStr, QPainterPath &path)
}
case 'S': {
if (count < 4) {
num += count;
count = 0;
ok = false;
break;
}
QPointF c1;
Expand All @@ -1803,8 +1797,7 @@ static bool parsePathDataFast(QStringView dataStr, QPainterPath &path)
}
case 'q': {
if (count < 4) {
num += count;
count = 0;
ok = false;
break;
}
QPointF c(num[0] + offsetX, num[1] + offsetY);
Expand All @@ -1819,8 +1812,7 @@ static bool parsePathDataFast(QStringView dataStr, QPainterPath &path)
}
case 'Q': {
if (count < 4) {
num += count;
count = 0;
ok = false;
break;
}
QPointF c(num[0], num[1]);
Expand All @@ -1835,8 +1827,7 @@ static bool parsePathDataFast(QStringView dataStr, QPainterPath &path)
}
case 't': {
if (count < 2) {
num += count;
count = 0;
ok = false;
break;
}
QPointF e(num[0] + offsetX, num[1] + offsetY);
Expand All @@ -1856,8 +1847,7 @@ static bool parsePathDataFast(QStringView dataStr, QPainterPath &path)
}
case 'T': {
if (count < 2) {
num += count;
count = 0;
ok = false;
break;
}
QPointF e(num[0], num[1]);
Expand All @@ -1877,8 +1867,7 @@ static bool parsePathDataFast(QStringView dataStr, QPainterPath &path)
}
case 'a': {
if (count < 7) {
num += count;
count = 0;
ok = false;
break;
}
qreal rx = (*num++);
Expand All @@ -1900,8 +1889,7 @@ static bool parsePathDataFast(QStringView dataStr, QPainterPath &path)
break;
case 'A': {
if (count < 7) {
num += count;
count = 0;
ok = false;
break;
}
qreal rx = (*num++);
Expand All @@ -1922,12 +1910,15 @@ static bool parsePathDataFast(QStringView dataStr, QPainterPath &path)
}
break;
default:
return false;
ok = false;
break;
}
lastMode = pathElem.toLatin1();
if (path.elementCount() > maxElementCount)
ok = false;
}
}
return true;
return ok;
}

static bool parseStyle(QSvgNode *node,
Expand Down Expand Up @@ -2985,8 +2976,8 @@ static QSvgNode *createPathNode(QSvgNode *parent,

QPainterPath qpath;
qpath.setFillRule(Qt::WindingFill);
//XXX do error handling
parsePathDataFast(data, qpath);
if (!parsePathDataFast(data, qpath))
qCWarning(lcSvgHandler, "Invalid path data; path truncated.");

QSvgNode *path = new QSvgPath(parent, qpath);
return path;
Expand Down

0 comments on commit 36cfd9e

Please sign in to comment.