Introduce a strict mode in KeePass2XmlReader.

Many errors are now ignored when not in strict mode so we can still parse
files that have been written by broken/incomplete implementations.
This commit is contained in:
Felix Geyer 2014-12-01 21:52:51 +01:00
parent 226c061c01
commit 71d39865b3
7 changed files with 127 additions and 16 deletions

View File

@ -35,9 +35,15 @@ KeePass2XmlReader::KeePass2XmlReader()
, m_db(Q_NULLPTR)
, m_meta(Q_NULLPTR)
, m_error(false)
, m_strictMode(false)
{
}
void KeePass2XmlReader::setStrictMode(bool strictMode)
{
m_strictMode = strictMode;
}
void KeePass2XmlReader::readDatabase(QIODevice* device, Database* db, KeePass2RandomStream* randomStream)
{
m_error = false;
@ -493,7 +499,12 @@ Group* KeePass2XmlReader::parseGroup()
if (m_xml.name() == "UUID") {
Uuid uuid = readUuid();
if (uuid.isNull()) {
raiseError("Null group uuid");
if (m_strictMode) {
raiseError("Null group uuid");
}
else {
group->setUuid(Uuid::random());
}
}
else {
group->setUuid(uuid);
@ -508,7 +519,9 @@ Group* KeePass2XmlReader::parseGroup()
else if (m_xml.name() == "IconID") {
int iconId = readNumber();
if (iconId < 0) {
raiseError("Invalid group icon number");
if (m_strictMode) {
raiseError("Invalid group icon number");
}
}
else {
if (iconId >= DatabaseIcons::IconCount) {
@ -584,6 +597,10 @@ Group* KeePass2XmlReader::parseGroup()
}
}
if (group->uuid().isNull() && !m_strictMode) {
group->setUuid(Uuid::random());
}
if (!group->uuid().isNull()) {
Group* tmpGroup = group;
group = getGroup(tmpGroup->uuid());
@ -630,7 +647,9 @@ void KeePass2XmlReader::parseDeletedObject()
if (m_xml.name() == "UUID") {
Uuid uuid = readUuid();
if (uuid.isNull()) {
raiseError("Null DeleteObject uuid");
if (m_strictMode) {
raiseError("Null DeleteObject uuid");
}
}
else {
delObj.uuid = uuid;
@ -647,7 +666,7 @@ void KeePass2XmlReader::parseDeletedObject()
if (!delObj.uuid.isNull() && !delObj.deletionTime.isNull()) {
m_db->addDeletedObject(delObj);
}
else {
else if (m_strictMode) {
raiseError("Missing DeletedObject uuid or time");
}
}
@ -665,7 +684,12 @@ Entry* KeePass2XmlReader::parseEntry(bool history)
if (m_xml.name() == "UUID") {
Uuid uuid = readUuid();
if (uuid.isNull()) {
raiseError("Null entry uuid");
if (m_strictMode) {
raiseError("Null entry uuid");
}
else {
entry->setUuid(Uuid::random());
}
}
else {
entry->setUuid(uuid);
@ -674,7 +698,9 @@ Entry* KeePass2XmlReader::parseEntry(bool history)
else if (m_xml.name() == "IconID") {
int iconId = readNumber();
if (iconId < 0) {
raiseError("Invalud entry icon number");
if (m_strictMode) {
raiseError("Invalud entry icon number");
}
}
else {
entry->setIcon(iconId);
@ -726,6 +752,10 @@ Entry* KeePass2XmlReader::parseEntry(bool history)
}
}
if (entry->uuid().isNull() && !m_strictMode) {
entry->setUuid(Uuid::random());
}
if (!entry->uuid().isNull()) {
if (history) {
entry->setUpdateTimeinfo(false);
@ -986,7 +1016,12 @@ QDateTime KeePass2XmlReader::readDateTime()
QDateTime dt = QDateTime::fromString(str, Qt::ISODate);
if (!dt.isValid()) {
raiseError("Invalid date time value");
if (m_strictMode) {
raiseError("Invalid date time value");
}
else {
dt = Tools::currentDateTimeUtc();
}
}
return dt;
@ -1001,7 +1036,9 @@ QColor KeePass2XmlReader::readColor()
}
if (colorStr.length() != 7 || colorStr[0] != '#') {
raiseError("Invalid color value");
if (m_strictMode) {
raiseError("Invalid color value");
}
return QColor();
}
@ -1011,7 +1048,9 @@ QColor KeePass2XmlReader::readColor()
bool ok;
int rgbPart = rgbPartStr.toInt(&ok, 16);
if (!ok || rgbPart > 255) {
raiseError("Invalid color rgb part");
if (m_strictMode) {
raiseError("Invalid color rgb part");
}
return QColor();
}
@ -1043,7 +1082,9 @@ Uuid KeePass2XmlReader::readUuid()
{
QByteArray uuidBin = readBinary();
if (uuidBin.length() != Uuid::Length) {
raiseError("Invalid uuid value");
if (m_strictMode) {
raiseError("Invalid uuid value");
}
return Uuid();
}
else {

View File

@ -47,6 +47,7 @@ public:
bool hasError();
QString errorString();
QByteArray headerHash();
void setStrictMode(bool strictMode);
private:
bool parseKeePassFile();
@ -95,6 +96,7 @@ private:
QByteArray m_headerHash;
bool m_error;
QString m_errorStr;
bool m_strictMode;
};
#endif // KEEPASSX_KEEPASS2XMLREADER_H

View File

@ -90,6 +90,7 @@ void TestDeletedObjects::createAndDelete(Database* db, int delObjectsSize)
void TestDeletedObjects::testDeletedObjectsFromFile()
{
KeePass2XmlReader reader;
reader.setStrictMode(true);
QString xmlFile = QString(KEEPASSX_TEST_DATA_DIR).append("/NewDatabase.xml");
Database* db = reader.readDatabase(xmlFile);

View File

@ -71,6 +71,7 @@ void TestKeePass2XmlReader::initTestCase()
QVERIFY(Crypto::init());
KeePass2XmlReader reader;
reader.setStrictMode(true);
QString xmlFile = QString(KEEPASSX_TEST_DATA_DIR).append("/NewDatabase.xml");
m_db = reader.readDatabase(xmlFile);
QVERIFY(m_db);
@ -357,23 +358,41 @@ void TestKeePass2XmlReader::testDeletedObjects()
void TestKeePass2XmlReader::testBroken()
{
QFETCH(QString, baseName);
QFETCH(bool, strictMode);
QFETCH(bool, expectError);
KeePass2XmlReader reader;
reader.setStrictMode(strictMode);
QString xmlFile = QString("%1/%2.xml").arg(KEEPASSX_TEST_DATA_DIR, baseName);
QVERIFY(QFile::exists(xmlFile));
QScopedPointer<Database> db(reader.readDatabase(xmlFile));
QVERIFY(reader.hasError());
if (reader.hasError()) {
qWarning("Reader error: %s", qPrintable(reader.errorString()));
}
QCOMPARE(reader.hasError(), expectError);
}
void TestKeePass2XmlReader::testBroken_data()
{
QTest::addColumn<QString>("baseName");
QTest::addColumn<bool>("strictMode");
QTest::addColumn<bool>("expectError");
QTest::newRow("BrokenNoGroupUuid") << "BrokenNoGroupUuid";
QTest::newRow("BrokenNoEntryUuid") << "BrokenNoEntryUuid";
QTest::newRow("BrokenNoRootGroup") << "BrokenNoRootGroup";
QTest::newRow("BrokenTwoRoots") << "BrokenTwoRoots";
QTest::newRow("BrokenTwoRootGroups") << "BrokenTwoRootGroups";
// testfile strict? error?
QTest::newRow("BrokenNoGroupUuid (strict)") << "BrokenNoGroupUuid" << true << true;
QTest::newRow("BrokenNoGroupUuid (not strict)") << "BrokenNoGroupUuid" << false << false;
QTest::newRow("BrokenNoEntryUuid (strict)") << "BrokenNoEntryUuid" << true << true;
QTest::newRow("BrokenNoEntryUuid (not strict)") << "BrokenNoEntryUuid" << false << false;
QTest::newRow("BrokenNoRootGroup (strict)") << "BrokenNoRootGroup" << true << true;
QTest::newRow("BrokenNoRootGroup (not strict)") << "BrokenNoRootGroup" << false << true;
QTest::newRow("BrokenTwoRoots (strict)") << "BrokenTwoRoots" << true << true;
QTest::newRow("BrokenTwoRoots (not strict)") << "BrokenTwoRoots" << false << true;
QTest::newRow("BrokenTwoRootGroups (strict)") << "BrokenTwoRootGroups" << true << true;
QTest::newRow("BrokenTwoRootGroups (not strict)") << "BrokenTwoRootGroups" << false << true;
QTest::newRow("BrokenGroupReference (strict)") << "BrokenGroupReference" << true << false;
QTest::newRow("BrokenGroupReference (not strict)") << "BrokenGroupReference" << false << false;
QTest::newRow("BrokenDeletedObjects (strict)") << "BrokenDeletedObjects" << true << true;
QTest::newRow("BrokenDeletedObjects (not strict)") << "BrokenDeletedObjects" << false << false;
}
void TestKeePass2XmlReader::cleanupTestCase()

View File

@ -0,0 +1,27 @@
<?xml version="1.0" encoding="utf-8" standalone="yes"?>
<KeePassFile>
<Root>
<Group>
<UUID>lmU+9n0aeESKZvcEze+bRg==</UUID>
<Name>Test</Name>
<Entry>
<UUID>AaUYVdXsI02h4T1RiAlgtg==</UUID>
<String>
<Key>Title</Key>
<Value>Sample Entry 1</Value>
</String>
</Entry>
</Group>
<DeletedObjects>
<DeletedObject>
<UUID/>
<DeletionTime>2010-08-25T16:14:12Z</DeletionTime>
</DeletedObject>
<DeletedObject/>
<DeletedObject>
<UUID>5K/bzWCSmkCv5OZxYl4N/w==</UUID>
<DeletionTime/>
</DeletedObject>
</DeletedObjects>
</Root>
</KeePassFile>

View File

@ -0,0 +1,20 @@
<?xml version="1.0" encoding="utf-8" standalone="yes"?>
<KeePassFile>
<Meta>
<RecycleBinEnabled>True</RecycleBinEnabled>
<RecycleBinUUID>6w7wZdhAp0qVlXjkemuCYw==</RecycleBinUUID>
</Meta>
<Root>
<Group>
<UUID>lmU+9n0aeESKZvcEze+bRg==</UUID>
<Name>Test</Name>
<Entry>
<UUID>AaUYVdXsI02h4T1RiAlgtg==</UUID>
<String>
<Key>Title</Key>
<Value>Sample Entry 1</Value>
</String>
</Entry>
</Group>
</Root>
</KeePassFile>

View File

@ -9,6 +9,7 @@
<Key>Title</Key>
<Value>Sample Entry 1</Value>
</String>
</Entry>
</Group>
</Root>
</KeePassFile>