From 2fa531745ff6e95afc8922a919b1acd070044022 Mon Sep 17 00:00:00 2001
From: Felix Geyer <debfx@fobos.de>
Date: Sun, 1 Nov 2015 18:30:50 +0100
Subject: [PATCH] Check XML key file for valid base64 before using it.

QByteArray::fromBase64() doesn't validate the input.

Closes #366
---
 src/core/Tools.cpp                     |  10 ++++++++++
 src/core/Tools.h                       |   1 +
 src/keys/FileKey.cpp                   |   5 ++++-
 tests/TestKeys.cpp                     |   1 +
 tests/data/FileKeyXmlBrokenBase64.kdbx | Bin 0 -> 1582 bytes
 tests/data/FileKeyXmlBrokenBase64.key  |   9 +++++++++
 6 files changed, 25 insertions(+), 1 deletion(-)
 create mode 100644 tests/data/FileKeyXmlBrokenBase64.kdbx
 create mode 100644 tests/data/FileKeyXmlBrokenBase64.key

diff --git a/src/core/Tools.cpp b/src/core/Tools.cpp
index 8034417f0..8ed083361 100644
--- a/src/core/Tools.cpp
+++ b/src/core/Tools.cpp
@@ -160,6 +160,16 @@ bool isHex(const QByteArray& ba)
     return true;
 }
 
+bool isBase64(const QByteArray& ba)
+{
+    QRegExp regexp("^(?:[a-z0-9+/]{4})*(?:[a-z0-9+/]{3}=|[a-z0-9+/]{2}==)?$",
+                   Qt::CaseInsensitive, QRegExp::RegExp2);
+
+    QString base64 = QString::fromLatin1(ba.constData(), ba.size());
+
+    return regexp.exactMatch(base64);
+}
+
 void sleep(int ms)
 {
     Q_ASSERT(ms >= 0);
diff --git a/src/core/Tools.h b/src/core/Tools.h
index 8058f2518..3854507b5 100644
--- a/src/core/Tools.h
+++ b/src/core/Tools.h
@@ -35,6 +35,7 @@ bool readAllFromDevice(QIODevice* device, QByteArray& data);
 QDateTime currentDateTimeUtc();
 QString imageReaderFilter();
 bool isHex(const QByteArray& ba);
+bool isBase64(const QByteArray& ba);
 void sleep(int ms);
 void wait(int ms);
 QString platform();
diff --git a/src/keys/FileKey.cpp b/src/keys/FileKey.cpp
index f03a69536..d399f545f 100644
--- a/src/keys/FileKey.cpp
+++ b/src/keys/FileKey.cpp
@@ -211,7 +211,10 @@ QByteArray FileKey::loadXmlKey(QXmlStreamReader& xmlReader)
     while (!xmlReader.error() && xmlReader.readNextStartElement()) {
         if (xmlReader.name() == "Data") {
             // TODO: do we need to enforce a specific data.size()?
-            data = QByteArray::fromBase64(xmlReader.readElementText().toLatin1());
+            QByteArray rawData = xmlReader.readElementText().toLatin1();
+            if (Tools::isBase64(rawData)) {
+                data = QByteArray::fromBase64(rawData);
+            }
         }
     }
 
diff --git a/tests/TestKeys.cpp b/tests/TestKeys.cpp
index 770af52de..b6617754a 100644
--- a/tests/TestKeys.cpp
+++ b/tests/TestKeys.cpp
@@ -113,6 +113,7 @@ void TestKeys::testFileKey_data()
 {
     QTest::addColumn<QString>("type");
     QTest::newRow("Xml") << QString("Xml");
+    QTest::newRow("XmlBrokenBase64") << QString("XmlBrokenBase64");
     QTest::newRow("Binary") << QString("Binary");
     QTest::newRow("Hex") << QString("Hex");
     QTest::newRow("Hashed") << QString("Hashed");
diff --git a/tests/data/FileKeyXmlBrokenBase64.kdbx b/tests/data/FileKeyXmlBrokenBase64.kdbx
new file mode 100644
index 0000000000000000000000000000000000000000..7c3ee30f523e6af3c77f81e98b18d2b71bbc3750
GIT binary patch
literal 1582
zcmV+}2GRKg*`k_f`%AR}00RI55CAd3^5(yBLr}h01tDtuTK@wC0096100bZa@@l8?
z0igSBgME#w%#h?Y!!kGv@kyQ-+yrDly>7Wv1t0(>VB#jR5qoUMz*tWCs>C8TW3>!t
z-!+L{MBc$2@#R1U2mo*w00000000LN0GHD)ZF#MgIrQ=sj2w-WX9yqwZF&iGA}a9X
zk*O$G|6O5BV?z->vy~&??o$hAGf==X2_OLTxpuP*#;R)AOz1LWAk5Yrg$yD~DTBi8
zRl}3~srWPs1ONg60000401XNa3X)pf-Xrz}DPOTn;{q6=KlR$WE%Dy9XcgU(!CX!H
z#Dt)SZ+h>oSab@H$1vaEw*MW0?9tm-9j|9C!E`E^WNRSNYB?R|CpFRqKTR)K?Ys<)
zxfQIkkr9IG8k?|u#kYo{JGGyT{}4t#OjWx>jR3^zHId6T={B|MB^1q;O0(nojcrU0
zc(Zl%W2uY7g8MY#u(trU&xZML`Qis!(Xt0(AjhGYYQSBs%I%XV4lA3?3x?*OAbGh@
zy+Fw>f$S37(n$J*qhjWluGm>8Riqq%DpHhyeVP7v2CkLW#1jm~V<N>Q@0K^=cg!$o
z>we=L9mw((BIDDJg#s~D+VB&2C02kR6$ax%%bioxz9hxaYwvw+TJQ%ddjuP-w2i^P
zsqrCo76p~{M(d>ZnG0Qe7w;P&GaOf#-^A>M<Ir+zU;em?M2kjd#MJEGOo7{vKBT^+
z4p>>fqq%D%$uBSeB?Qbv{AR;DXO^U~khC65mC<*y;zF!OSh@a`tHDii?D0R(G|grQ
z<*Z~x6@I7;fK&u5U6oCDD`c7!5c=B9(aNNSoe{9EG_V<3aS0rV2~zaOWn}K7jQG6$
zrV%yJzr_=cy{xZ|g1sqMZz}JV6Q!%}o<g~$hU;cTw;XF6^rt#ko{=2fetWGza~@va
zb23C7be)!3b26J2JtD2yav;w34w+aU92(1!7J+a5=cwncx8<$V1hDu?_flO%m*?)(
zE=KYF5DxpRK5Kh2+uymkS*4gjp7&30m9AALxy9&nh(n+e1|Pg`6Lzm`y>YhT)=C=e
zXI&!IJV$8L&lgg5^K>B{c~o~<SgsP4zHqau!>IK)I8Za=9_<;s*b5&6iS};>{F;v!
zrsDiy4(w^^hm}5Q&PfEWgzMneI^(K_j;1#%pmWL(4W9W`&h{Lnv4M&ufowP~W=e>t
zk8f*Az0BCHER}6PULQe_eBcJXSrRipXn;yvS?f@2!8w9s*w(T<5vs=$w<ucDLLMTq
z&hB)`kf!1_fO`xbnzptHmgIiYgM0$?rx=B+s!COKpQ<;uHcLo$Dlz#Zr550uuLL4+
zO#7HA^eb(eJ)K>>*UieqJIksfOD37}X&Dpt>BV%|2qlr-l5h9T!Ra4gy*V{!nImf3
zyA~l3<y#gvgex!`v}`Ke=nb{TL8i!P9$pcEZO5I8cGo|e`?1;c+_5XK&=5~abDaVZ
zQ5k@RN`W5#gJAp1&71FYe3YlH(3IB9i`k5Y<@D{k=BRFiV<rSo-$)Ra{djUqXxVMj
zLiy=qD9{8;E$v&tI_0LJVQK0;R++_pxY}8^ZV66O5C4f6sP_{T-QN4MY+2qIcpVA_
z#Hr;mG#S7m!?kDj57kkmndJPvmr0W-Ai=@92p#k(`Q|5<OkDo9i71<;6PinAqHq%Z
zW~kgFjti){dJax+%?w}VKx7%MxVVN!FYUXC=Y>SER#7Cjbp@UYBE(~eo@!<fhS`BH
ztEdk?j>3W~$ar2&$n_SRP5%@UYVJTB1{&N>Fay4KWgP9LhRgvG0?rWEW9bmat8SAX
z;xBI|ub0~y)#g-g#9OgTX2Am<LYm?mDk&N8LJUH>kGH!%u%IiN51rC~Y;OAb^@>e-
z45R2E*>dXJ4n+!hC8nF5ocNx6?ws05O}BK3AlE!4nHk14MS7sL%D~@6Bl{hfn#exX
z65|S3{3$;V$)NT0ItdJqf@1nq_}dXri}L<J9zML%mBMGVK9b=Vzmp%k|9o+Jt2kc5
g>jMW+i6BvIBLHHZ*Wh$mS75xkkN)fM?r6s*6p|wXfB*mh

literal 0
HcmV?d00001

diff --git a/tests/data/FileKeyXmlBrokenBase64.key b/tests/data/FileKeyXmlBrokenBase64.key
new file mode 100644
index 000000000..530ecec22
--- /dev/null
+++ b/tests/data/FileKeyXmlBrokenBase64.key
@@ -0,0 +1,9 @@
+<?xml version="1.0" encoding="utf-8"?>
+<KeyFile>
+	<Meta>
+		<Version>1.00</Version>
+	</Meta>
+	<Key>
+		<Data>yy</Data>
+	</Key>
+</KeyFile>