qml: escape untrusted text in remaining RichText views

XMR

Monero Project/Monero GUI

3d3a391e View source ↗

Public commit record

What the developer wrote

Authored by Thomas

85/100 · Strong

qml: escape untrusted text in remaining RichText views

Extends the escaping from commit 23ec5eb6 to the RichText sinks it did
not cover: the transaction note in the tx details popup (History), the
wallet name and account label in the send confirmation
(TxConfirmationDialog), the address label on the merchant page
(Merchant), and the wallet path on the info page (SettingsInfo). These
were interpolated unescaped, so a value containing markup is rendered as
rich text.

The transaction note is the notable case: it can be set from a payment
request's tx_description, so it is attacker influenced.

Escape these fields with Utils.htmlEscape. Set the send confirmation
From field to Text.RichText explicitly so the escaped entities decode in
both of its branches; the single-account branch contains no tag and
would otherwise render as plain text and show the raw entity.

✓ Specific, descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context✓ Explains rationale or failure mode

The short version

What changed, and why it matters

This commit fixes a bug where user-controlled text was being displayed as rich text in several places in the Monero wallet app. Rich text can include hidden instructions, fake links, or misleading formatting. The most important case is the transaction note, which can be supplied by someone sending you a payment request, so an attacker could potentially use it to trick you. The fix escapes that text so it is shown as plain characters rather than interpreted as formatting or code.

Recommended action

Review the commit for completeness and confirm no other RichText sinks interpolate untrusted data. Consider auditing all QML Text.RichText usages in the codebase for similar escaping gaps. No immediate user action is required beyond updating to the patched version.

Security signals we found

RichText injection / UI spoofing via unescaped attacker-influenced strings

Cross-branch rendering inconsistency fixed by explicit textFormat

Follow-up to prior escaping fix (commit 23ec5eb6), indicating a class of similar sinks

Technical analysis

Evidence from the diff

The patch applies Utils.htmlEscape to four QML RichText sinks that previously interpolated untrusted strings directly: wallet name and account label in TxConfirmationDialog, transaction note in History, address label in Merchant, and wallet path in SettingsInfo. The transaction note is attacker-influenced via tx_description in payment requests. Without escaping, malicious markup or HTML entities in these values would be rendered by Qt’s Text.RichText engine, enabling UI spoofing or possibly link injection. The patch also explicitly sets textFormat: Text.RichText in the send-confirmation From field so escaped entities decode consistently in both branches.

Changed components

components/TxConfirmationDialog.qmlpages/History.qmlpages/merchant/Merchant.qmlpages/settings/SettingsInfo.qml

Inspect captured patch +6 / −5

diff --git a/components/TxConfirmationDialog.qml b/components/TxConfirmationDialog.qml
index 1700aa0..51da022 100644
--- a/components/TxConfirmationDialog.qml
+++ b/components/TxConfirmationDialog.qml
@@ -242,6 +242,7 @@ Rectangle {
                     Layout.fillWidth: true
                     font.pixelSize: 15
                     color: MoneroComponents.Style.defaultFontColor
+                    textFormat: Text.RichText
                     text: {
                         if (currentWallet) {
                             var walletTitle = function() {
@@ -257,9 +258,9 @@ Rectangle {
                             if (appWindow.currentWallet.numSubaddressAccounts() > 1) {
                                 var currentSubaddressAccount = currentWallet.currentSubaddressAccount;
                                 var currentAccountLabel =  currentWallet.getSubaddressLabel(currentWallet.currentSubaddressAccount, 0);
-                                return walletTitle() + " (" + walletName + ")" + "<br>" + qsTr("Account #") + currentSubaddressAccount + (currentAccountLabel !== "" ? " (" + currentAccountLabel + ")" : "") + translationManager.emptyString;
+                                return walletTitle() + " (" + Utils.htmlEscape(walletName) + ")" + "<br>" + qsTr("Account #") + currentSubaddressAccount + (currentAccountLabel !== "" ? " (" + Utils.htmlEscape(currentAccountLabel) + ")" : "") + translationManager.emptyString;
                             } else {
-                                return walletTitle() + " (" + walletName + ")" + translationManager.emptyString;
+                                return walletTitle() + " (" + Utils.htmlEscape(walletName) + ")" + translationManager.emptyString;
                             }
                         } else {
                             return "";
diff --git a/pages/History.qml b/pages/History.qml
index d9132e3..6871405 100644
--- a/pages/History.qml
+++ b/pages/History.qml
@@ -1746,7 +1746,7 @@ Rectangle {
             + (paymentId ? trStart + qsTr("Payment ID:") + trMiddle + paymentId + trEnd : "")
             + (integratedAddress ? trStart + qsTr("Integrated address") + ":" + trMiddle + integratedAddress + trEnd : "")
             + (tx_key ? trStart + qsTr("Tx key:") + trMiddle + tx_key + trEnd : "")
-            + (tx_note ? trStart + qsTr("Tx note:") + trMiddle + tx_note + trEnd : "")
+            + (tx_note ? trStart + qsTr("Tx note:") + trMiddle + Utils.htmlEscape(tx_note) + trEnd : "")
             + (destinations ? trStart + qsTr("Destinations:") + trMiddle + destinations + trEnd : "")
             + (rings ? trStart + qsTr("Rings:") + trMiddle + rings + trEnd : "")
             + "</table>"
diff --git a/pages/merchant/Merchant.qml b/pages/merchant/Merchant.qml
index 560484b..86d4756 100644
--- a/pages/merchant/Merchant.qml
+++ b/pages/merchant/Merchant.qml
@@ -295,7 +295,7 @@ Item {
                     color: "white"
                     text: "<style type='text/css'>a {text-decoration: none; color: #FF6C3C; font-size: 12px;}</style>%1: %2 <a href='#'>(%3)</a>"
                         .arg(qsTr("Currently selected address"))
-                        .arg(addressLabel)
+                        .arg(Utils.htmlEscape(addressLabel))
                         .arg(qsTr("Change")) + translationManager.emptyString
                     textFormat: Text.RichText
                     themeTransition: false
diff --git a/pages/settings/SettingsInfo.qml b/pages/settings/SettingsInfo.qml
index ad152d6..e126383 100644
--- a/pages/settings/SettingsInfo.qml
+++ b/pages/settings/SettingsInfo.qml
@@ -142,7 +142,7 @@ Rectangle {
                     <style type='text/css'>\
                         a {cursor:pointer;text-decoration: none; color: #FF6C3C}\
                     </style>\
-                    <a href='#'>%1</a>".arg(walletPath)
+                    <a href='#'>%1</a>".arg(Utils.htmlEscape(walletPath))
                 textFormat: Text.RichText
                 onLinkActivated: oshelper.openContainingFolder(walletPath)

Risk score

Why this scored 66/100

Our methodology →

Potential impact 18/30

Exploitability 16/25

Stealth signal 10/15

Affected reach 10/15

Confidence 8/10

Evidence quality 4/5

Human-validated context

Community notes

Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.

No validated notes yet.

The AI analysis stands alone for now. Submit a note if you can add evidence or important context.

What the developer wrote

What changed, and why it matters

Security signals we found

Evidence from the diff

Changed components

Why this scored 66/100

Community notes

Related watches

Merge pull request #4645

Merge pull request #4687

Merge pull request #4709