From 98115b13b7a674ebac9d91384e509587397782fb Mon Sep 17 00:00:00 2001 From: Raul Metsma Date: Wed, 14 Jan 2026 14:08:00 +0200 Subject: [PATCH] Do not allow cancel in PinPad view and clean PIN memory IB-7082 Signed-off-by: Raul Metsma --- client/QSmartCard.cpp | 94 ++++++++++++++++++++----------------- client/QSmartCard_p.h | 20 ++++---- client/dialogs/PinPopup.cpp | 84 +++++++++++++++++---------------- client/dialogs/PinPopup.h | 11 ++--- 4 files changed, 110 insertions(+), 99 deletions(-) diff --git a/client/QSmartCard.cpp b/client/QSmartCard.cpp index 0607f13c4..531775b6d 100644 --- a/client/QSmartCard.cpp +++ b/client/QSmartCard.cpp @@ -92,9 +92,12 @@ const QByteArray Card::READBINARY = APDU("00B00000 00"); const QByteArray Card::REPLACE = APDU("002C0000 00"); const QByteArray Card::VERIFY = APDU("00200000 00"); -QPCSCReader::Result Card::transfer(QPCSCReader *reader, bool verify, const QByteArray &apdu, +QPCSCReader::Result Card::transfer(QPCSCReader *reader, bool verify, QByteArray &&apdu, QSmartCardData::PinType type, quint8 newPINOffset, bool requestCurrentPIN) { + auto clean = qScopeGuard([&apdu] { + apdu.fill('0'); + }); if(!reader->isPinPad()) return reader->transfer(apdu); quint16 language = 0x0000; @@ -119,6 +122,17 @@ QByteArrayView Card::parseFCI(QByteArrayView data, quint8 expectedTag) return {}; } +QByteArray Card::pinTemplate(const QString &data) const +{ + QByteArray pin = data.toUtf8(); +#if QT_VERSION >= QT_VERSION_CHECK(6, 4, 0) + pin.resize(12, fillChar); +#else + pin += QByteArray(12 - pin.size(), fillChar); +#endif + return pin; +} + struct TLV { quint32 tag {}; @@ -191,11 +205,9 @@ const QByteArray IDEMIACard::AID_QSCD = APDU("00A4040C 10 51534344204170706C6963 const QByteArray IDEMIACard::ATR_COSMO8 = QByteArrayLiteral("3BDB960080B1FE451F830012233F536549440F9000F1"); const QByteArray IDEMIACard::ATR_COSMOX = QByteArrayLiteral("3BDC960080B1FE451F830012233F54654944320F9000C3"); -QPCSCReader::Result IDEMIACard::change(QPCSCReader *reader, QSmartCardData::PinType type, QByteArray &&pin, QByteArray &&newpin) const +QPCSCReader::Result IDEMIACard::change(QPCSCReader *reader, QSmartCardData::PinType type, const QByteArray &pin, const QByteArray &newpin) const { QByteArray cmd = CHANGE; - newpin = pinTemplate(std::move(newpin)); - pin = pinTemplate(std::move(pin)); switch (type) { case QSmartCardData::Pin1Type: cmd[3] = 1; @@ -210,7 +222,9 @@ QPCSCReader::Result IDEMIACard::change(QPCSCReader *reader, QSmartCardData::PinT break; } cmd[4] = char(pin.size() + newpin.size()); - return transfer(reader, false, cmd + pin + newpin, type, quint8(pin.size()), true); + cmd += pin; + cmd += newpin; + return transfer(reader, false, std::move(cmd), type, quint8(pin.size()), true); } bool IDEMIACard::isSupported(const QByteArray &atr) @@ -291,23 +305,13 @@ bool IDEMIACard::loadPerso(QPCSCReader *reader, QSmartCardDataPrivate *d) const return updateCounters(reader, d); } -QByteArray IDEMIACard::pinTemplate(QByteArray &&pin) -{ -#if QT_VERSION >= QT_VERSION_CHECK(6, 4, 0) - pin.resize(12, char(0xFF)); -#else - pin += QByteArray(12 - pin.size(), char(0xFF)); -#endif - return std::move(pin); -} - -QPCSCReader::Result IDEMIACard::replace(QPCSCReader *reader, QSmartCardData::PinType type, QByteArray &&puk, QByteArray &&pin) const +QPCSCReader::Result IDEMIACard::replace(QPCSCReader *reader, QSmartCardData::PinType type, const QByteArray &puk, const QByteArray &pin) const { - puk = pinTemplate(std::move(puk)); QByteArray cmd = VERIFY; cmd[3] = 2; cmd[4] = char(puk.size()); - if(auto result = transfer(reader, true, cmd + puk, QSmartCardData::PukType, 0, true); !result) + cmd += puk; + if(auto result = transfer(reader, true, std::move(cmd), QSmartCardData::PukType, 0, true); !result) return result; cmd = Card::REPLACE; @@ -320,9 +324,9 @@ QPCSCReader::Result IDEMIACard::replace(QPCSCReader *reader, QSmartCardData::Pin } else cmd[3] = char(type); - pin = pinTemplate(std::move(pin)); cmd[4] = char(pin.size()); - return transfer(reader, false, cmd + pin, type, 0, false); + cmd += pin; + return transfer(reader, false, std::move(cmd), type, 0, false); } bool IDEMIACard::updateCounters(QPCSCReader *reader, QSmartCardDataPrivate *d) const @@ -351,14 +355,14 @@ bool IDEMIACard::updateCounters(QPCSCReader *reader, QSmartCardDataPrivate *d) c const QByteArray THALESCard::AID = APDU("00A4040C 0C A000000063504B43532D3135"); -QPCSCReader::Result THALESCard::change(QPCSCReader *reader, QSmartCardData::PinType type, QByteArray &&pin, QByteArray &&newpin) const +QPCSCReader::Result THALESCard::change(QPCSCReader *reader, QSmartCardData::PinType type, const QByteArray &pin, const QByteArray &newpin) const { QByteArray cmd = CHANGE; - newpin = pinTemplate(std::move(newpin)); - pin = pinTemplate(std::move(pin)); cmd[3] = char(0x80 | type); cmd[4] = char(pin.size() + newpin.size()); - return transfer(reader, false, cmd + pin + newpin, type, quint8(pin.size()), true); + cmd += pin; + cmd += newpin; + return transfer(reader, false, std::move(cmd), type, quint8(pin.size()), true); } bool THALESCard::isSupported(const QByteArray &atr) @@ -441,25 +445,14 @@ bool THALESCard::loadPerso(QPCSCReader *reader, QSmartCardDataPrivate *d) const return updateCounters(reader, d); } -QByteArray THALESCard::pinTemplate(const QString &pin) -{ - QByteArray result = pin.toUtf8(); -#if QT_VERSION >= QT_VERSION_CHECK(6, 4, 0) - result.resize(12, char(0x00)); -#else - result += QByteArray(12 - result.size(), char(0x00)); -#endif - return result; -} - -QPCSCReader::Result THALESCard::replace(QPCSCReader *reader, QSmartCardData::PinType type, QByteArray &&puk, QByteArray &&pin) const +QPCSCReader::Result THALESCard::replace(QPCSCReader *reader, QSmartCardData::PinType type, const QByteArray &puk, const QByteArray &pin) const { - puk = pinTemplate(std::move(puk)); - pin = pinTemplate(std::move(pin)); QByteArray cmd = REPLACE; cmd[3] = char(0x80 | type); cmd[4] = char(puk.size() + pin.size()); - return transfer(reader, false, cmd + puk + pin, type, quint8(puk.size()), true); + cmd += puk; + cmd += pin; + return transfer(reader, false, std::move(cmd), type, quint8(puk.size()), true); } bool THALESCard::updateCounters(QPCSCReader *reader, QSmartCardDataPrivate *d) const @@ -533,6 +526,8 @@ bool QSmartCard::pinChange(QSmartCardData::PinType type, QSmartCard::PinAction a } popup.reset(new PinPopup(src, flags, d->t.authCert(), parent, bodyText)); popup->open(); + oldPin = d->card->pinTemplate({}); + newPin = d->card->pinTemplate({}); } else { @@ -540,9 +535,22 @@ bool QSmartCard::pinChange(QSmartCardData::PinType type, QSmartCard::PinAction a d->t.data(QSmartCardData::Id).toString(), d->t.isPUKReplacable(), parent); if (!p.exec()) return false; - oldPin = p.firstCodeText().toUtf8(); - newPin = p.newCodeText().toUtf8(); + QString oldPinString = p.firstCodeText(); + QString newPinString = p.newCodeText(); + oldPin = d->card->pinTemplate(oldPinString); + newPin = d->card->pinTemplate(newPinString); + // Try to clean QLineEdit internal PIN copy using constData that does not detach memory + auto chars = const_cast(oldPinString.constData()); + for (int i = 0; i < oldPinString.length(); ++i) + chars[i] = '\0'; + chars = const_cast(newPinString.constData()); + for (int i = 0; i < newPinString.length(); ++i) + chars[i] = '\0'; } + auto clean = qScopeGuard([&oldPin, &newPin] { + oldPin.fill('\0'); + newPin.fill('\0'); + }); QPCSCReader reader(d->t.reader(), &QPCSC::instance()); if(!reader.connect()) @@ -551,8 +559,8 @@ bool QSmartCard::pinChange(QSmartCardData::PinType type, QSmartCard::PinAction a return false; } auto response = action == QSmartCard::ChangeWithPin || action == QSmartCard::ActivateWithPin ? - d->card->change(&reader, type, std::move(oldPin), std::move(newPin)) : - d->card->replace(&reader, type, std::move(oldPin), std::move(newPin)); + d->card->change(&reader, type, oldPin, newPin) : + d->card->replace(&reader, type, oldPin, newPin); switch(response.SW) { case 0x9000: diff --git a/client/QSmartCard_p.h b/client/QSmartCard_p.h index 6015f2510..dc83e6c37 100644 --- a/client/QSmartCard_p.h +++ b/client/QSmartCard_p.h @@ -33,10 +33,11 @@ class Card { public: virtual ~Card() noexcept = default; - virtual QPCSCReader::Result change(QPCSCReader *reader, QSmartCardData::PinType type, QByteArray &&pin, QByteArray &&newpin) const = 0; + virtual QPCSCReader::Result change(QPCSCReader *reader, QSmartCardData::PinType type, const QByteArray &pin, const QByteArray &newpin) const = 0; virtual bool loadPerso(QPCSCReader *reader, QSmartCardDataPrivate *d) const = 0; - virtual QPCSCReader::Result replace(QPCSCReader *reader, QSmartCardData::PinType type, QByteArray &&puk, QByteArray &&pin) const = 0; - static QPCSCReader::Result transfer(QPCSCReader *reader, bool verify, const QByteArray &apdu, + virtual QByteArray pinTemplate(const QString &pin) const; + virtual QPCSCReader::Result replace(QPCSCReader *reader, QSmartCardData::PinType type, const QByteArray &puk, const QByteArray &pin) const = 0; + static QPCSCReader::Result transfer(QPCSCReader *reader, bool verify, QByteArray &&apdu, QSmartCardData::PinType type, quint8 newPINOffset, bool requestCurrentPIN); virtual bool updateCounters(QPCSCReader *reader, QSmartCardDataPrivate *d) const = 0; @@ -46,18 +47,20 @@ class Card static const QByteArray READBINARY; static const QByteArray REPLACE; static const QByteArray VERIFY; + + char fillChar = 0x00; }; class IDEMIACard: public Card { public: - QPCSCReader::Result change(QPCSCReader *reader, QSmartCardData::PinType type, QByteArray &&pin, QByteArray &&newpin) const final; + IDEMIACard() { fillChar = 0xFF; } + QPCSCReader::Result change(QPCSCReader *reader, QSmartCardData::PinType type, const QByteArray &pin, const QByteArray &newpin) const final; bool loadPerso(QPCSCReader *reader, QSmartCardDataPrivate *d) const final; - QPCSCReader::Result replace(QPCSCReader *reader, QSmartCardData::PinType type, QByteArray &&puk, QByteArray &&pin) const final; + QPCSCReader::Result replace(QPCSCReader *reader, QSmartCardData::PinType type, const QByteArray &puk, const QByteArray &pin) const final; bool updateCounters(QPCSCReader *reader, QSmartCardDataPrivate *d) const final; static bool isSupported(const QByteArray &atr); - static QByteArray pinTemplate(QByteArray &&pin); static const QByteArray AID, AID_OT, AID_QSCD, ATR_COSMO8, ATR_COSMOX; }; @@ -65,13 +68,12 @@ class IDEMIACard: public Card class THALESCard: public Card { public: - QPCSCReader::Result change(QPCSCReader *reader, QSmartCardData::PinType type, QByteArray &&pin, QByteArray &&newpin) const final; + QPCSCReader::Result change(QPCSCReader *reader, QSmartCardData::PinType type, const QByteArray &pin, const QByteArray &newpin) const final; bool loadPerso(QPCSCReader *reader, QSmartCardDataPrivate *d) const final; - QPCSCReader::Result replace(QPCSCReader *reader, QSmartCardData::PinType type, QByteArray &&puk, QByteArray &&pin) const final; + QPCSCReader::Result replace(QPCSCReader *reader, QSmartCardData::PinType type, const QByteArray &puk, const QByteArray &pin) const final; bool updateCounters(QPCSCReader *reader, QSmartCardDataPrivate *d) const final; static bool isSupported(const QByteArray &atr); - static QByteArray pinTemplate(const QString &pin); static const QByteArray AID; }; diff --git a/client/dialogs/PinPopup.cpp b/client/dialogs/PinPopup.cpp index ed77a6196..ef90b511e 100644 --- a/client/dialogs/PinPopup.cpp +++ b/client/dialogs/PinPopup.cpp @@ -24,36 +24,34 @@ #include #include -#include #include -#include #include PinPopup::PinPopup(QSmartCardData::PinType type, TokenFlags flags, const SslCertificate &c, QWidget *parent, QString text) : QDialog(parent) - , ui(new Ui::PinPopup) { - ui->setupUi(this); + Ui::PinPopup ui {}; + ui.setupUi(this); setWindowFlags(Qt::Dialog | Qt::CustomizeWindowHint); #ifdef Q_OS_WIN - ui->buttonLayout->setDirection(QBoxLayout::RightToLeft); + ui.buttonLayout->setDirection(QBoxLayout::RightToLeft); #endif for(QLineEdit *w: findChildren()) w->setAttribute(Qt::WA_MacShowFocusRect, false); - ui->pin->setValidator(regexp = new QRegularExpressionValidator(ui->pin)); + ui.pin->setValidator(regexp = new QRegularExpressionValidator(ui.pin)); new Overlay(this); - connect( ui->ok, &QPushButton::clicked, this, &PinPopup::accept ); - connect( ui->cancel, &QPushButton::clicked, this, &PinPopup::reject ); - connect( this, &PinPopup::finished, this, &PinPopup::close ); - connect(ui->pin, &QLineEdit::returnPressed, ui->ok, &QPushButton::click); + connect(ui.ok, &QPushButton::clicked, this, &PinPopup::accept); + connect(ui.cancel, &QPushButton::clicked, this, &PinPopup::reject); + connect(this, &PinPopup::finished, this, &PinPopup::close ); + connect(ui.pin, &QLineEdit::returnPressed, ui.ok, &QPushButton::click); if(!text.isEmpty()) - ui->labelPin->hide(); + ui.labelPin->hide(); else if(type == QSmartCardData::Pin2Type) { text = tr("Selected action requires sign certificate."); - ui->labelPin->setText(flags & PinpadFlag ? + ui.labelPin->setText(flags & PinpadFlag ? tr("For using sign certificate enter PIN2 at the reader") : tr("For using sign certificate enter PIN2")); setPinLen(5); @@ -61,43 +59,45 @@ PinPopup::PinPopup(QSmartCardData::PinType type, TokenFlags flags, const SslCert else { text = tr("Selected action requires authentication certificate."); - ui->labelPin->setText(flags & PinpadFlag ? + ui.labelPin->setText(flags & PinpadFlag ? tr("For using authentication certificate enter PIN1 at the reader") : tr("For using authentication certificate enter PIN1")); setPinLen(4); } - ui->label->setText(c.toString(c.showCN() ? QStringLiteral("CN, serialNumber") : QStringLiteral("GN SN, serialNumber"))); - ui->text->setText(text); + ui.label->setText(c.toString(c.showCN() ? QStringLiteral("CN, serialNumber") : QStringLiteral("GN SN, serialNumber"))); + ui.text->setText(text); if(flags & PinFinalTry) - ui->errorPin->setText(tr("%1 will be locked next failed attempt").arg(QSmartCardData::typeString(type))); + ui.errorPin->setText(tr("%1 will be locked next failed attempt").arg(QSmartCardData::typeString(type))); else if(flags & PinCountLow) - ui->errorPin->setText(tr("%1 has been entered incorrectly at least once").arg(QSmartCardData::typeString(type))); + ui.errorPin->setText(tr("%1 has been entered incorrectly at least once").arg(QSmartCardData::typeString(type))); else - ui->errorPin->hide(); + ui.errorPin->hide(); if(flags & PinpadChangeFlag) { - ui->pin->hide(); - ui->ok->hide(); - ui->cancel->hide(); - ui->errorPin->setAlignment(Qt::AlignCenter); + isPinPad = true; + ui.pin->hide(); + ui.ok->hide(); + ui.cancel->hide(); + ui.errorPin->setAlignment(Qt::AlignCenter); auto *movie = new QSvgWidget(QStringLiteral(":/images/wait.svg"), this); - movie->setFixedSize(ui->pin->size().height(), ui->pin->size().height()); + movie->setFixedSize(ui.pin->size().height(), ui.pin->size().height()); movie->show(); - ui->layoutContent->addWidget(movie, 0, Qt::AlignCenter); + ui.layoutContent->addWidget(movie, 0, Qt::AlignCenter); } else if(flags & PinpadFlag) { - ui->pin->hide(); - ui->ok->hide(); - ui->cancel->hide(); + isPinPad = true; + ui.pin->hide(); + ui.ok->hide(); + ui.cancel->hide(); auto *progress = new QProgressBar(this); progress->setRange( 0, 30 ); progress->setValue( progress->maximum() ); progress->setTextVisible( false ); progress->resize( 200, 30 ); progress->move( 153, 122 ); - ui->layoutContent->addWidget(progress); + ui.layoutContent->addWidget(progress); auto *statusTimer = new QTimeLine(progress->maximum() * 1000, this); statusTimer->setEasingCurve(QEasingCurve::Linear); statusTimer->setFrameRange( progress->maximum(), progress->minimum() ); @@ -106,24 +106,20 @@ PinPopup::PinPopup(QSmartCardData::PinType type, TokenFlags flags, const SslCert } else { - ui->pin->setFocus(); - ui->pin->setMaxLength( 12 ); - connect(ui->pin, &QLineEdit::textEdited, this, [&](const QString &text) { - ui->ok->setEnabled(regexp->regularExpression().match(text).hasMatch()); + ui.pin->setFocus(); + ui.pin->setMaxLength( 12 ); + connect(ui.pin, &QLineEdit::textEdited, this, [&](const QString &text) { + ui.ok->setEnabled(regexp->regularExpression().match(text).hasMatch()); }); - ui->text->setBuddy( ui->pin ); - ui->ok->setDisabled(true); + ui.text->setBuddy(ui.pin); + ui.ok->setDisabled(true); } if(c.type() & SslCertificate::TempelType) { regexp->setRegularExpression(QRegularExpression(QStringLiteral("^.{4,}$"))); - ui->pin->setMaxLength(32767); + ui.pin->setMaxLength(32767); } -} - -PinPopup::~PinPopup() -{ - delete ui; + pinInput = ui.pin; } void PinPopup::setPinLen(unsigned long minLen, unsigned long maxLen) @@ -132,4 +128,10 @@ void PinPopup::setPinLen(unsigned long minLen, unsigned long maxLen) regexp->setRegularExpression(QRegularExpression(QStringLiteral("^%1{%2,%3}$").arg(charPattern).arg(minLen).arg(maxLen))); } -QString PinPopup::pin() const { return ui->pin->text(); } +QString PinPopup::pin() const { return pinInput->text(); } + +void PinPopup::reject() +{ + if (!isPinPad) + QDialog::reject(); +} diff --git a/client/dialogs/PinPopup.h b/client/dialogs/PinPopup.h index ce866d51b..08f8d9e3a 100644 --- a/client/dialogs/PinPopup.h +++ b/client/dialogs/PinPopup.h @@ -24,10 +24,7 @@ #include "QSmartCard.h" #include "WaitDialog.h" -namespace Ui { -class PinPopup; -} - +class QLineEdit; class QRegularExpressionValidator; class SslCertificate; @@ -47,7 +44,6 @@ class PinPopup final : public QDialog Q_DECLARE_FLAGS(TokenFlags, TokenFlag) PinPopup(QSmartCardData::PinType type, TokenFlags flags, const SslCertificate &cert, QWidget *parent = {}, QString text = {}); - ~PinPopup() final; void setPinLen(unsigned long minLen, unsigned long maxLen = 12); QString pin() const; @@ -56,8 +52,11 @@ class PinPopup final : public QDialog void startTimer(); private: - Ui::PinPopup *ui; + void reject() final; + + QLineEdit *pinInput; QRegularExpressionValidator *regexp {}; WaitDialogHider hider; + bool isPinPad = false; };