From f9d26960462349196306a4ce4815978e5268ec47 Mon Sep 17 00:00:00 2001 From: Jonathan White Date: Fri, 8 Nov 2019 18:06:13 -0500 Subject: [PATCH] Relax strictness of TOTP Base32 validation * Fix #3754 - Accept valid TOTP keys that require padding when converted to Base32. * Allow use of spaces and lower case letters in the TOTP secret key. --- src/gui/TotpSetupDialog.cpp | 13 ++++++++----- tests/gui/TestGui.cpp | 5 +++-- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/gui/TotpSetupDialog.cpp b/src/gui/TotpSetupDialog.cpp index 8acf7d115..b350bedc4 100644 --- a/src/gui/TotpSetupDialog.cpp +++ b/src/gui/TotpSetupDialog.cpp @@ -1,6 +1,5 @@ /* - * Copyright (C) 2017 Weslly Honorato <weslly@protonmail.com> - * Copyright (C) 2017 KeePassXC Team + * Copyright (C) 2019 KeePassXC Team * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -46,9 +45,11 @@ TotpSetupDialog::~TotpSetupDialog() void TotpSetupDialog::saveSettings() { // Secret key sanity check - auto key = m_ui->seedEdit->text().toLatin1(); + // Convert user input to all uppercase and remove '=' + auto key = m_ui->seedEdit->text().toUpper().remove(" ").remove("=").toLatin1(); auto sanitizedKey = Base32::sanitizeInput(key); - if (sanitizedKey != key) { + // Use startsWith to ignore added '=' for padding at the end + if (!sanitizedKey.startsWith(key)) { MessageBox::information(this, tr("Invalid TOTP Secret"), tr("You have entered an invalid secret key. The key must be in Base32 format.\n" @@ -112,7 +113,9 @@ void TotpSetupDialog::init() // Read entry totp settings auto settings = m_entry->totpSettings(); if (settings) { - m_ui->seedEdit->setText(settings->key); + auto key = settings->key; + m_ui->seedEdit->setText(key.remove("=")); + m_ui->seedEdit->setCursorPosition(0); m_ui->stepSpinBox->setValue(settings->step); if (settings->encoder.shortName == Totp::STEAM_SHORTNAME) { diff --git a/tests/gui/TestGui.cpp b/tests/gui/TestGui.cpp index ca208db01..2a0bef483 100644 --- a/tests/gui/TestGui.cpp +++ b/tests/gui/TestGui.cpp @@ -756,7 +756,8 @@ void TestGui::testTotp() QApplication::processEvents(); - QString exampleSeed = "gezdgnbvgy3tqojqgezdgnbvgy3tqojq"; + QString exampleSeed = "gezd gnbvgY 3tqojqGEZdgnb vgy3tqoJq==="; + QString expectedFinalSeed = exampleSeed.toUpper().remove(" ").remove("="); auto* seedEdit = setupTotpDialog->findChild("seedEdit"); seedEdit->setText(""); QTest::keyClicks(seedEdit, exampleSeed); @@ -781,7 +782,7 @@ void TestGui::testTotp() editEntryWidget->setCurrentPage(1); auto* attrTextEdit = editEntryWidget->findChild("attributesEdit"); QTest::mouseClick(editEntryWidget->findChild("revealAttributeButton"), Qt::LeftButton); - QCOMPARE(attrTextEdit->toPlainText(), exampleSeed); + QCOMPARE(attrTextEdit->toPlainText(), expectedFinalSeed); auto* editEntryWidgetButtonBox = editEntryWidget->findChild("buttonBox"); QTest::mouseClick(editEntryWidgetButtonBox->button(QDialogButtonBox::Ok), Qt::LeftButton);