Skip to content

Commit

Permalink
uic: Relax property name checks
Browse files Browse the repository at this point in the history
Use an approximation of the "Unicode Standard Annex qt#31"
for checking the property names and enumerations.

Remove check for dynamic properties and use character literal
formatting for them instead, which will escape offending characters.

As a drive-by, use QStringView parameters for the check function.

Amends 53c8d1f.

Task-number: QTBUG-126265
Task-number: QTBUG-126860
Pick-to: 6.8 6.7 6.5 6.2 5.15
Change-Id: I90fe555e64327e4164a17c1af0a734e4b1d834db
Reviewed-by: Edward Welbourne <[email protected]>
Reviewed-by: Jarek Kobus <[email protected]>
  • Loading branch information
FriedemannKleint committed Jul 8, 2024
1 parent da2d3e9 commit aae20da
Showing 1 changed file with 53 additions and 22 deletions.
75 changes: 53 additions & 22 deletions src/tools/uic/cpp/cppwriteinitialization.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -125,35 +125,65 @@ namespace {
return iconHasStatePixmaps(i) || !i->attributeTheme().isEmpty();
}

// Checks on property names
bool isIdentifier(QChar c) { return c.isLetterOrNumber() || c == u'_'; }

bool checkPropertyName(const QString &name)
// An approximation of "Unicode Standard Annex #31" for checking property
// and enumeration identifiers to prevent code injection attacks.
// FIXME 6.9: Simplify according to QTBUG-126860
static bool isIdStart(QChar c)
{
return !name.isEmpty() && name.at(0).isLetter()
&& std::all_of(name.cbegin(), name.cend(), isIdentifier);
bool result = false;
switch (c.category()) {
case QChar::Letter_Uppercase:
case QChar::Letter_Lowercase:
case QChar::Letter_Titlecase:
case QChar::Letter_Modifier:
case QChar::Letter_Other:
case QChar::Number_Letter:
result = true;
break;
default:
result = c == u'_';
break;
}
return result;
}

// Basic checks on enum/flag values
static bool isValidEnumValue(QChar c)
static bool isIdContinuation(QChar c)
{
if (c.isLetterOrNumber())
return true;
switch (c.unicode()) {
case '|':
case ' ':
case ':':
case '_':
return true;
bool result = false;
switch (c.category()) {
case QChar::Letter_Uppercase:
case QChar::Letter_Lowercase:
case QChar::Letter_Titlecase:
case QChar::Letter_Modifier:
case QChar::Letter_Other:
case QChar::Number_Letter:
case QChar::Mark_NonSpacing:
case QChar::Mark_SpacingCombining:
case QChar::Number_DecimalDigit:
case QChar::Punctuation_Connector: // '_'
result = true;
break;
default:
break;
}
return false;
return result;
}

static bool isEnumIdContinuation(QChar c)
{
return c == u':' || c == u'|' || c == u' ' || isIdContinuation(c);
}

bool checkPropertyName(QStringView name)
{
return !name.isEmpty() && isIdStart(name.at(0))
&& std::all_of(name.cbegin() + 1, name.cend(), isIdContinuation);
}

bool checkEnumValue(const QString &value)
bool checkEnumValue(QStringView name)
{
return std::all_of(value.cbegin(), value.cend(), isValidEnumValue);
return !name.isEmpty() && isIdStart(name.at(0))
&& std::all_of(name.cbegin() + 1, name.cend(), isEnumIdContinuation);
}

QString msgInvalidValue(const QString &name, const QString &value)
Expand All @@ -169,7 +199,8 @@ namespace {
const DomProperty *p) {

const QString &name = p->attributeName();
if (!checkPropertyName(name)) {
const bool isDynamicProperty = p->hasAttributeStdset() && p->attributeStdset() == 0;
if (!isDynamicProperty && !checkPropertyName(name)) {
qWarning("uic: Invalid property name: \"%s\".", qPrintable(name));
return false;
}
Expand Down Expand Up @@ -1403,8 +1434,8 @@ void WriteInitialization::writeProperties(const QString &varName,
str << language::derefPointer <<"set" << propertyName.at(0).toUpper()
<< QStringView{propertyName}.mid(1) << '(';
} else {
str << language::derefPointer << "setProperty(\""_L1
<< propertyName << "\", ";
str << language::derefPointer << "setProperty("_L1
<< language::charliteral(propertyName) << ", ";
if (language::language() == Language::Cpp) {
str << "QVariant";
if (p->kind() == DomProperty::Enum)
Expand Down

0 comments on commit aae20da

Please sign in to comment.