From 2d741afe3e025442ecbb37e1d3b8adbb9b09994c Mon Sep 17 00:00:00 2001
From: Felix Geyer <debfx@fobos.de>
Date: Sun, 24 Jan 2016 17:20:16 +0100
Subject: [PATCH] Strip invalid XML chars when writing databases.

These characters are unprintable or just plain invalid.
QXmlStreamReader throws and error when reading XML documents with such chars.

Closes #392
---
 src/format/KeePass2XmlWriter.cpp | 21 +++++++++++++++++++--
 src/format/KeePass2XmlWriter.h   |  1 +
 tests/TestKeePass2XmlReader.cpp  | 31 +++++++++++++++++++++++++++++++
 tests/TestKeePass2XmlReader.h    |  1 +
 4 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/src/format/KeePass2XmlWriter.cpp b/src/format/KeePass2XmlWriter.cpp
index 313607c2..0d83935a 100644
--- a/src/format/KeePass2XmlWriter.cpp
+++ b/src/format/KeePass2XmlWriter.cpp
@@ -374,7 +374,7 @@ void KeePass2XmlWriter::writeEntry(const Entry* entry)
         }
 
         if (!value.isEmpty()) {
-            m_xml.writeCharacters(value);
+            m_xml.writeCharacters(stripInvalidXml10Chars(value));
         }
         m_xml.writeEndElement();
 
@@ -445,7 +445,7 @@ void KeePass2XmlWriter::writeString(const QString& qualifiedName, const QString&
         m_xml.writeEmptyElement(qualifiedName);
     }
     else {
-        m_xml.writeTextElement(qualifiedName, string);
+        m_xml.writeTextElement(qualifiedName, stripInvalidXml10Chars(string));
     }
 }
 
@@ -549,6 +549,23 @@ QString KeePass2XmlWriter::colorPartToString(int value)
     return str;
 }
 
+QString KeePass2XmlWriter::stripInvalidXml10Chars(QString str)
+{
+    for (int i = str.size() - 1; i >= 0; i--) {
+        const ushort uc = str.at(i).unicode();
+
+        if ((uc < 0x20 && uc != 0x09 && uc != 0x0A && uc != 0x0D)
+                || (uc > 0xD7FF && uc < 0xE000)
+                || (uc > 0xFFFD))
+        {
+            qWarning("Stripping invalid XML 1.0 codepoint %x", uc);
+            str.remove(i, 1);
+        }
+    }
+
+    return str;
+}
+
 void KeePass2XmlWriter::raiseError(const QString& errorMessage)
 {
     m_error = true;
diff --git a/src/format/KeePass2XmlWriter.h b/src/format/KeePass2XmlWriter.h
index ea621244..48dde6af 100644
--- a/src/format/KeePass2XmlWriter.h
+++ b/src/format/KeePass2XmlWriter.h
@@ -73,6 +73,7 @@ private:
     void writeColor(const QString& qualifiedName, const QColor& color);
     void writeTriState(const QString& qualifiedName, Group::TriState triState);
     QString colorPartToString(int value);
+    QString stripInvalidXml10Chars(QString str);
 
     void raiseError(const QString& errorMessage);
 
diff --git a/tests/TestKeePass2XmlReader.cpp b/tests/TestKeePass2XmlReader.cpp
index 81690009..f8ca2961 100644
--- a/tests/TestKeePass2XmlReader.cpp
+++ b/tests/TestKeePass2XmlReader.cpp
@@ -17,6 +17,7 @@
 
 #include "TestKeePass2XmlReader.h"
 
+#include <QBuffer>
 #include <QFile>
 #include <QTest>
 
@@ -26,6 +27,7 @@
 #include "core/Metadata.h"
 #include "crypto/Crypto.h"
 #include "format/KeePass2XmlReader.h"
+#include "format/KeePass2XmlWriter.h"
 #include "config-keepassx-tests.h"
 
 QTEST_GUILESS_MAIN(TestKeePass2XmlReader)
@@ -408,6 +410,35 @@ void TestKeePass2XmlReader::testEmptyUuids()
     QVERIFY(!reader.hasError());
 }
 
+void TestKeePass2XmlReader::testInvalidXmlChars()
+{
+    QScopedPointer<Database> dbWrite(new Database());
+
+    Entry* entry = new Entry();
+    entry->setUuid(Uuid::random());
+    entry->setNotes(QString("a %1 b %2 c %3").arg(QChar(0x02)).arg(QChar(0xD800)).arg(QChar(0xFFFE)));
+    entry->setGroup(dbWrite->rootGroup());
+
+    QBuffer buffer;
+    buffer.open(QIODevice::ReadWrite);
+    KeePass2XmlWriter writer;
+    writer.writeDatabase(&buffer, dbWrite.data());
+    QVERIFY(!writer.hasError());
+    buffer.seek(0);
+
+    KeePass2XmlReader reader;
+    reader.setStrictMode(true);
+    QScopedPointer<Database> dbRead(reader.readDatabase(&buffer));
+    if (reader.hasError()) {
+        qWarning("Database read error: %s", qPrintable(reader.errorString()));
+    }
+    QVERIFY(!reader.hasError());
+    QVERIFY(!dbRead.isNull());
+    QCOMPARE(dbRead->rootGroup()->entries().size(), 1);
+    // check that the invalid codepoints have been stripped
+    QCOMPARE(dbRead->rootGroup()->entries().first()->notes(), QString("a  b  c "));
+}
+
 void TestKeePass2XmlReader::cleanupTestCase()
 {
     delete m_db;
diff --git a/tests/TestKeePass2XmlReader.h b/tests/TestKeePass2XmlReader.h
index 651ac561..815dbcd6 100644
--- a/tests/TestKeePass2XmlReader.h
+++ b/tests/TestKeePass2XmlReader.h
@@ -42,6 +42,7 @@ private Q_SLOTS:
     void testBroken();
     void testBroken_data();
     void testEmptyUuids();
+    void testInvalidXmlChars();
     void cleanupTestCase();
 
 private: