diff options
| author | Danilo M. <danix@danix.xyz> | 2026-09-18 16:04:37 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-09-18 16:04:37 +0200 |
| commit | 7454096cae63d87c75a676751bb07f5650280cd0 (patch) | |
| tree | ff2d1b2c60f25bbdb5419c9a9259e7d4f160e2e5 /src | |
| parent | f7815097104ae6da9bb763d1b63df55217818445 (diff) | |
| download | qtmaildir-7454096cae63d87c75a676751bb07f5650280cd0.tar.gz qtmaildir-7454096cae63d87c75a676751bb07f5650280cd0.zip | |
fix: sanitise untrusted contact names on insertion
A vCard FN is untrusted and ContactStore faithfully decodes `\n` to a real
newline, so `Evil\nBcc: x` was inserted raw into a recipient field and flowed
through splitRecipients() to MessageBuilder. contactInsertionText() only quoted
a name carrying a comma or a double quote, so every other RFC 5322 special
(<, >, ;, @) and any control character reached the header unguarded.
The name now has control characters and whitespace runs replaced by single
spaces, and every non-empty name is quoted, with `\` escaped before `"`.
Quoting contains all the specials in one step. The parser is left faithful;
this is fixed at the consumer/trust boundary.
Tests: a name with a decoded newline inserts no control character and yields
one recipient; a name with <, >, ;, @ is quoted and yields one recipient. The
three tests that expected an unquoted plain name now expect the quoted form.
Diffstat (limited to 'src')
| -rw-r--r-- | src/composewindow.cpp | 40 |
1 files changed, 24 insertions, 16 deletions
diff --git a/src/composewindow.cpp b/src/composewindow.cpp index f62f38f..eb983d6 100644 --- a/src/composewindow.cpp +++ b/src/composewindow.cpp @@ -124,26 +124,34 @@ QStringList splitRecipients(const QString &text) /// The text a completion inserts for \p contact. /// -/// A display name containing a comma must be QUOTED, because the recipient -/// fields are comma-separated and splitRecipients() would otherwise cut the -/// name in half before the message was built. A name containing a double quote -/// is quoted too, with its quotes backslash-escaped: an unquoted `"` in an -/// address header is malformed. The backslash is escaped FIRST, or escaping the -/// quote would then double the backslashes it just introduced. +/// UNTRUSTED INPUT ARRIVES HERE. A display name comes from a vCard's FN, and +/// ContactStore::unescapeText() faithfully decodes `\n` to a real newline, so a +/// hostile card can carry `Evil\nBcc: x` and be inserted raw into a recipient +/// header. The name is sanitised and then ALWAYS quoted when non-empty, rather +/// than only for a comma or a quote: quoting contains every RFC 5322 special +/// (`<`, `>`, `;`, `@`, spaces, commas) in one step, and escaping `\` before `"` +/// is what makes the quoted form valid. Control characters become single spaces +/// and runs collapse, so a decoded newline cannot forge a header line. QString contactInsertionText(const Contact &contact) { - if (contact.name.isEmpty()) - return contact.email; - - if (contact.name.contains(QLatin1Char(',')) - || contact.name.contains(QLatin1Char('"'))) { - QString escaped = contact.name; - escaped.replace(QLatin1Char('\\'), QStringLiteral("\\\\")); - escaped.replace(QLatin1Char('"'), QStringLiteral("\\\"")); - return QStringLiteral("\"%1\" <%2>").arg(escaped, contact.email); + QString name; + name.reserve(contact.name.size()); + for (const QChar c : contact.name) { + if (c.isSpace() || c.category() == QChar::Other_Control) { + if (!name.isEmpty() && !name.endsWith(QLatin1Char(' '))) + name.append(QLatin1Char(' ')); + continue; + } + name.append(c); } + name = name.trimmed(); + + if (name.isEmpty()) + return contact.email; - return QStringLiteral("%1 <%2>").arg(contact.name, contact.email); + name.replace(QLatin1Char('\\'), QStringLiteral("\\\\")); + name.replace(QLatin1Char('"'), QStringLiteral("\\\"")); + return QStringLiteral("\"%1\" <%2>").arg(name, contact.email); } /// The span an accepted completion replaces: the comma-delimited token the |
