uic: Prevent code injection via invalid property names/values

Fixes: QTBUG-126265 Pick-to: 6.8 6.7 6.5 6.2 5.15 Change-Id: Id0d6706b8565b76fcc7b9e73944dc6d3e8232e49 Reviewed-by: Jarek Kobus <[email protected]>
quadtrees · Jun 13, 2024 · 53c8d1f · 53c8d1f
1 parent 756e4d4
commit 53c8d1f
Show file tree

Hide file tree

Showing 3 changed files with 163 additions and 4 deletions.
diff --git a/src/tools/uic/cpp/cppwriteinitialization.cpp b/src/tools/uic/cpp/cppwriteinitialization.cpp
@@ -127,16 +127,63 @@ 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)
+    {
+        return !name.isEmpty() && name.at(0).isLetter()
+               && std::all_of(name.cbegin(), name.cend(), isIdentifier);
+    }
+
+    // Basic checks on enum/flag values
+    static bool isValidEnumValue(QChar c)
+    {
+        if (c.isLetterOrNumber())
+            return true;
+        switch (c.unicode()) {
+        case '|':
+        case ' ':
+        case ':':
+        case '_':
+            return true;
+        default:
+            break;
+        }
+        return false;
+    }
+
+    bool checkEnumValue(const QString &value)
+    {
+        return std::all_of(value.cbegin(), value.cend(), isValidEnumValue);
+    }
+
+    QString msgInvalidValue(const QString &name, const QString &value)
+    {
+        return "uic: Invalid property value: \""_L1 + name + "\": \""_L1 + value + u'"';
+    }
+
     // Check on properties. Filter out empty legacy pixmap/icon properties
     // as Designer pre 4.4 used to remove missing resource references.
     // This can no longer be handled by the code as we have 'setIcon(QIcon())' as well as 'QIcon icon'
     static bool checkProperty(const CustomWidgetsInfo *customWidgetsInfo,
                               const QString &fileName, const QString &className,
                               const DomProperty *p) {
+
+        const QString &name = p->attributeName();
+        if (!checkPropertyName(name)) {
+            qWarning("uic: Invalid property name: \"%s\".", qPrintable(name));
+            return false;
+        }
+
         switch (p->kind()) {
         // ### fixme Qt 7 remove this: Exclude deprecated properties of Qt 5.
         case DomProperty::Set:
-            if (p->attributeName() == u"features"
+            if (!checkEnumValue(p->elementSet())) {
+                qWarning("%s", qPrintable(msgInvalidValue(name, p->elementSet())));
+                return false;
+            }
+            if (name == u"features"
                 && customWidgetsInfo->extends(className, "QDockWidget")
                 && p->elementSet() == u"QDockWidget::AllDockWidgetFeatures") {
                 const QString msg = fileName + ": Warning: Deprecated enum value QDockWidget::AllDockWidgetFeatures was encountered."_L1;
@@ -145,7 +192,11 @@ namespace {
             }
             break;
         case DomProperty::Enum:
-            if (p->attributeName() == u"sizeAdjustPolicy"
+            if (!checkEnumValue(p->elementEnum())) {
+                qWarning("%s", qPrintable(msgInvalidValue(name, p->elementEnum())));
+                return false;
+            }
+            if (name == u"sizeAdjustPolicy"
                 && customWidgetsInfo->extends(className, "QComboBox")
                 && p->elementEnum() == u"QComboBox::AdjustToMinimumContentsLength") {
                 const QString msg = fileName + ": Warning: Deprecated enum value QComboBox::AdjustToMinimumContentsLength was encountered."_L1;
@@ -158,7 +209,7 @@ namespace {
                 if (!isIconFormat44(dri)) {
                     if (dri->text().isEmpty())  {
                         const QString msg = QString::fromLatin1("%1: Warning: An invalid icon property '%2' was encountered.")
-                                            .arg(fileName, p->attributeName());
+                                            .arg(fileName, name);
                         qWarning("%s", qPrintable(msg));
                         return false;
                     }
@@ -169,7 +220,7 @@ namespace {
             if (const DomResourcePixmap *drp = p->elementPixmap())
                 if (drp->text().isEmpty()) {
                     const QString msg = QString::fromUtf8("%1: Warning: An invalid pixmap property '%2' was encountered.")
-                                        .arg(fileName, p->attributeName());
+                                        .arg(fileName, name);
                     qWarning("%s", qPrintable(msg));
                     return false;
                 }

diff --git a/tests/auto/tools/uic/baseline/qtbug126265.ui b/tests/auto/tools/uic/baseline/qtbug126265.ui
@@ -0,0 +1,51 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<ui version="4.0">
+ <class>Form</class>
+ <widget class="QWidget" name="Form">
+  <property name="geometry">
+   <rect>
+    <x>0</x>
+    <y>0</y>
+    <width>400</width>
+    <height>300</height>
+   </rect>
+  </property>
+  <property name="windowTitle">
+   <string>Form</string>
+  </property>
+  <widget class="QPushButton" name="pushButton">
+   <property name="geometry">
+    <rect>
+     <x>70</x>
+     <y>40</y>
+     <width>91</width>
+     <height>29</height>
+    </rect>
+   </property>
+   <property name="text(&quot;Hello world!&quot;); injected code;//">
+    <enum>Qt::FocusPolicy::WheelFocus</enum>
+   </property>
+   <property name="text">
+    <string>PushButton</string>
+   </property>
+  </widget>
+  <widget class="QPushButton" name="pushButton_2">
+   <property name="geometry">
+    <rect>
+     <x>70</x>
+     <y>110</y>
+     <width>91</width>
+     <height>29</height>
+    </rect>
+   </property>
+   <property name="focusPolicy">
+    <enum>Qt::FocusPolicy::WheelFocus); injected code;//</enum>
+   </property>
+   <property name="text">
+    <string>PushButton</string>
+   </property>
+  </widget>
+ </widget>
+ <resources/>
+ <connections/>
+</ui>
diff --git a/tests/auto/tools/uic/baseline/qtbug126265.ui.h b/tests/auto/tools/uic/baseline/qtbug126265.ui.h
@@ -0,0 +1,57 @@
+/********************************************************************************
+** Form generated from reading UI file 'qtbug126265.ui'
+**
+** Created by: Qt User Interface Compiler version 6.0.0
+**
+** WARNING! All changes made in this file will be lost when recompiling UI file!
+********************************************************************************/
+
+#ifndef QTBUG126265_H
+#define QTBUG126265_H
+
+#include <QtCore/QVariant>
+#include <QtWidgets/QApplication>
+#include <QtWidgets/QPushButton>
+#include <QtWidgets/QWidget>
+
+QT_BEGIN_NAMESPACE
+
+class Ui_Form
+{
+public:
+    QPushButton *pushButton;
+    QPushButton *pushButton_2;
+
+    void setupUi(QWidget *Form)
+    {
+        if (Form->objectName().isEmpty())
+            Form->setObjectName("Form");
+        Form->resize(400, 300);
+        pushButton = new QPushButton(Form);
+        pushButton->setObjectName("pushButton");
+        pushButton->setGeometry(QRect(70, 40, 91, 29));
+        pushButton_2 = new QPushButton(Form);
+        pushButton_2->setObjectName("pushButton_2");
+        pushButton_2->setGeometry(QRect(70, 110, 91, 29));
+
+        retranslateUi(Form);
+
+        QMetaObject::connectSlotsByName(Form);
+    } // setupUi
+
+    void retranslateUi(QWidget *Form)
+    {
+        Form->setWindowTitle(QCoreApplication::translate("Form", "Form", nullptr));
+        pushButton->setText(QCoreApplication::translate("Form", "PushButton", nullptr));
+        pushButton_2->setText(QCoreApplication::translate("Form", "PushButton", nullptr));
+    } // retranslateUi
+
+};
+
+namespace Ui {
+    class Form: public Ui_Form {};
+} // namespace Ui
+
+QT_END_NAMESPACE
+
+#endif // QTBUG126265_H