Skip to content

Commit 7eeb909

Browse files
metsmametsma
committed
Do not allow cancel in PinPad view and clean PIN memory
IB-7082 Signed-off-by: Raul Metsma <[email protected]>
1 parent e82f2a6 commit 7eeb909

File tree

4 files changed

+93
-80
lines changed

4 files changed

+93
-80
lines changed

‎client/QSmartCard.cpp‎

Lines changed: 35 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -92,9 +92,12 @@ const QByteArray Card::READBINARY = APDU("00B00000 00");
9292
const QByteArray Card::REPLACE = APDU("002C0000 00");
9393
const QByteArray Card::VERIFY = APDU("00200000 00");
9494

95-
QPCSCReader::Result Card::transfer(QPCSCReader *reader, bool verify, const QByteArray &apdu,
95+
QPCSCReader::Result Card::transfer(QPCSCReader *reader, bool verify, QByteArray &&apdu,
9696
QSmartCardData::PinType type, quint8 newPINOffset, bool requestCurrentPIN)
9797
{
98+
auto clean = qScopeGuard([&apdu] {
99+
apdu.fill('0');
100+
});
98101
if(!reader->isPinPad())
99102
return reader->transfer(apdu);
100103
quint16 language = 0x0000;
@@ -191,11 +194,9 @@ const QByteArray IDEMIACard::AID_QSCD = APDU("00A4040C 10 51534344204170706C6963
191194
const QByteArray IDEMIACard::ATR_COSMO8 = QByteArrayLiteral("3BDB960080B1FE451F830012233F536549440F9000F1");
192195
const QByteArray IDEMIACard::ATR_COSMOX = QByteArrayLiteral("3BDC960080B1FE451F830012233F54654944320F9000C3");
193196

194-
QPCSCReader::Result IDEMIACard::change(QPCSCReader *reader, QSmartCardData::PinType type, QByteArray &&pin, QByteArray &&newpin) const
197+
QPCSCReader::Result IDEMIACard::change(QPCSCReader *reader, QSmartCardData::PinType type, const QByteArray &pin, const QByteArray &newpin) const
195198
{
196199
QByteArray cmd = CHANGE;
197-
newpin = pinTemplate(std::move(newpin));
198-
pin = pinTemplate(std::move(pin));
199200
switch (type) {
200201
case QSmartCardData::Pin1Type:
201202
cmd[3] = 1;
@@ -291,19 +292,19 @@ bool IDEMIACard::loadPerso(QPCSCReader *reader, QSmartCardDataPrivate *d) const
291292
return updateCounters(reader, d);
292293
}
293294

294-
QByteArray IDEMIACard::pinTemplate(QByteArray &&pin)
295+
QByteArray IDEMIACard::pinTemplate(const QString &data) const
295296
{
297+
QByteArray pin = data.toUtf8();
296298
#if QT_VERSION >= QT_VERSION_CHECK(6, 4, 0)
297299
pin.resize(12, char(0xFF));
298300
#else
299301
pin += QByteArray(12 - pin.size(), char(0xFF));
300302
#endif
301-
return std::move(pin);
303+
return pin;
302304
}
303305

304-
QPCSCReader::Result IDEMIACard::replace(QPCSCReader *reader, QSmartCardData::PinType type, QByteArray &&puk, QByteArray &&pin) const
306+
QPCSCReader::Result IDEMIACard::replace(QPCSCReader *reader, QSmartCardData::PinType type, const QByteArray &puk, const QByteArray &pin) const
305307
{
306-
puk = pinTemplate(std::move(puk));
307308
QByteArray cmd = VERIFY;
308309
cmd[3] = 2;
309310
cmd[4] = char(puk.size());
@@ -320,7 +321,6 @@ QPCSCReader::Result IDEMIACard::replace(QPCSCReader *reader, QSmartCardData::Pin
320321
}
321322
else
322323
cmd[3] = char(type);
323-
pin = pinTemplate(std::move(pin));
324324
cmd[4] = char(pin.size());
325325
return transfer(reader, false, cmd + pin, type, 0, false);
326326
}
@@ -351,11 +351,9 @@ bool IDEMIACard::updateCounters(QPCSCReader *reader, QSmartCardDataPrivate *d) c
351351

352352
const QByteArray THALESCard::AID = APDU("00A4040C 0C A000000063504B43532D3135");
353353

354-
QPCSCReader::Result THALESCard::change(QPCSCReader *reader, QSmartCardData::PinType type, QByteArray &&pin, QByteArray &&newpin) const
354+
QPCSCReader::Result THALESCard::change(QPCSCReader *reader, QSmartCardData::PinType type, const QByteArray &pin, const QByteArray &newpin) const
355355
{
356356
QByteArray cmd = CHANGE;
357-
newpin = pinTemplate(std::move(newpin));
358-
pin = pinTemplate(std::move(pin));
359357
cmd[3] = char(0x80 | type);
360358
cmd[4] = char(pin.size() + newpin.size());
361359
return transfer(reader, false, cmd + pin + newpin, type, quint8(pin.size()), true);
@@ -441,21 +439,19 @@ bool THALESCard::loadPerso(QPCSCReader *reader, QSmartCardDataPrivate *d) const
441439
return updateCounters(reader, d);
442440
}
443441

444-
QByteArray THALESCard::pinTemplate(const QString &pin)
442+
QByteArray THALESCard::pinTemplate(const QString &data) const
445443
{
446-
QByteArray result = pin.toUtf8();
444+
QByteArray pin = data.toUtf8();
447445
#if QT_VERSION >= QT_VERSION_CHECK(6, 4, 0)
448-
result.resize(12, char(0x00));
446+
pin.resize(12, char(0x00));
449447
#else
450-
result += QByteArray(12 - result.size(), char(0x00));
448+
pin += QByteArray(12 - result.size(), char(0x00));
451449
#endif
452-
return result;
450+
return pin;
453451
}
454452

455-
QPCSCReader::Result THALESCard::replace(QPCSCReader *reader, QSmartCardData::PinType type, QByteArray &&puk, QByteArray &&pin) const
453+
QPCSCReader::Result THALESCard::replace(QPCSCReader *reader, QSmartCardData::PinType type, const QByteArray &puk, const QByteArray &pin) const
456454
{
457-
puk = pinTemplate(std::move(puk));
458-
pin = pinTemplate(std::move(pin));
459455
QByteArray cmd = REPLACE;
460456
cmd[3] = char(0x80 | type);
461457
cmd[4] = char(puk.size() + pin.size());
@@ -533,16 +529,31 @@ bool QSmartCard::pinChange(QSmartCardData::PinType type, QSmartCard::PinAction a
533529
}
534530
popup.reset(new PinPopup(src, flags, d->t.authCert(), parent, bodyText));
535531
popup->open();
532+
oldPin = d->card->pinTemplate({});
533+
newPin = d->card->pinTemplate({});
536534
}
537535
else
538536
{
539537
PinUnblock p(type, action, d->t.retryCount(src), d->t.data(QSmartCardData::BirthDate).toDate(),
540538
d->t.data(QSmartCardData::Id).toString(), d->t.isPUKReplacable(), parent);
541539
if (!p.exec())
542540
return false;
543-
oldPin = p.firstCodeText().toUtf8();
544-
newPin = p.newCodeText().toUtf8();
541+
QString oldPinString = p.firstCodeText();
542+
QString newPinString = p.newCodeText();
543+
oldPin = d->card->pinTemplate(oldPinString);
544+
newPin = d->card->pinTemplate(newPinString);
545+
// Try to clean QLineEdit internal PIN copy using constData that does not detach memory
546+
auto chars = const_cast<QChar*>(oldPinString.constData());
547+
for (int i = 0; i < oldPinString.length(); ++i)
548+
chars[i] = '\0';
549+
chars = const_cast<QChar*>(newPinString.constData());
550+
for (int i = 0; i < newPinString.length(); ++i)
551+
chars[i] = '\0';
545552
}
553+
auto clean = qScopeGuard([&oldPin, &newPin] {
554+
oldPin.fill('\0');
555+
newPin.fill('\0');
556+
});
546557

547558
QPCSCReader reader(d->t.reader(), &QPCSC::instance());
548559
if(!reader.connect())
@@ -551,8 +562,8 @@ bool QSmartCard::pinChange(QSmartCardData::PinType type, QSmartCard::PinAction a
551562
return false;
552563
}
553564
auto response = action == QSmartCard::ChangeWithPin || action == QSmartCard::ActivateWithPin ?
554-
d->card->change(&reader, type, std::move(oldPin), std::move(newPin)) :
555-
d->card->replace(&reader, type, std::move(oldPin), std::move(newPin));
565+
d->card->change(&reader, type, oldPin, newPin) :
566+
d->card->replace(&reader, type, oldPin, newPin);
556567
switch(response.SW)
557568
{
558569
case 0x9000:

‎client/QSmartCard_p.h‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,11 @@ class Card
3333
{
3434
public:
3535
virtual ~Card() noexcept = default;
36-
virtual QPCSCReader::Result change(QPCSCReader *reader, QSmartCardData::PinType type, QByteArray &&pin, QByteArray &&newpin) const = 0;
36+
virtual QPCSCReader::Result change(QPCSCReader *reader, QSmartCardData::PinType type, const QByteArray &pin, const QByteArray &newpin) const = 0;
3737
virtual bool loadPerso(QPCSCReader *reader, QSmartCardDataPrivate *d) const = 0;
38-
virtual QPCSCReader::Result replace(QPCSCReader *reader, QSmartCardData::PinType type, QByteArray &&puk, QByteArray &&pin) const = 0;
39-
static QPCSCReader::Result transfer(QPCSCReader *reader, bool verify, const QByteArray &apdu,
38+
virtual QByteArray pinTemplate(const QString &pin) const = 0;
39+
virtual QPCSCReader::Result replace(QPCSCReader *reader, QSmartCardData::PinType type, const QByteArray &puk, const QByteArray &pin) const = 0;
40+
static QPCSCReader::Result transfer(QPCSCReader *reader, bool verify, QByteArray &&apdu,
4041
QSmartCardData::PinType type, quint8 newPINOffset, bool requestCurrentPIN);
4142
virtual bool updateCounters(QPCSCReader *reader, QSmartCardDataPrivate *d) const = 0;
4243

@@ -51,27 +52,27 @@ class Card
5152
class IDEMIACard: public Card
5253
{
5354
public:
54-
QPCSCReader::Result change(QPCSCReader *reader, QSmartCardData::PinType type, QByteArray &&pin, QByteArray &&newpin) const final;
55+
QPCSCReader::Result change(QPCSCReader *reader, QSmartCardData::PinType type, const QByteArray &pin, const QByteArray &newpin) const final;
5556
bool loadPerso(QPCSCReader *reader, QSmartCardDataPrivate *d) const final;
56-
QPCSCReader::Result replace(QPCSCReader *reader, QSmartCardData::PinType type, QByteArray &&puk, QByteArray &&pin) const final;
57+
QByteArray pinTemplate(const QString &pin) const final;
58+
QPCSCReader::Result replace(QPCSCReader *reader, QSmartCardData::PinType type, const QByteArray &puk, const QByteArray &pin) const final;
5759
bool updateCounters(QPCSCReader *reader, QSmartCardDataPrivate *d) const final;
5860

5961
static bool isSupported(const QByteArray &atr);
60-
static QByteArray pinTemplate(QByteArray &&pin);
6162

6263
static const QByteArray AID, AID_OT, AID_QSCD, ATR_COSMO8, ATR_COSMOX;
6364
};
6465

6566
class THALESCard: public Card
6667
{
6768
public:
68-
QPCSCReader::Result change(QPCSCReader *reader, QSmartCardData::PinType type, QByteArray &&pin, QByteArray &&newpin) const final;
69+
QPCSCReader::Result change(QPCSCReader *reader, QSmartCardData::PinType type, const QByteArray &pin, const QByteArray &newpin) const final;
6970
bool loadPerso(QPCSCReader *reader, QSmartCardDataPrivate *d) const final;
70-
QPCSCReader::Result replace(QPCSCReader *reader, QSmartCardData::PinType type, QByteArray &&puk, QByteArray &&pin) const final;
71+
QByteArray pinTemplate(const QString &pin) const final;
72+
QPCSCReader::Result replace(QPCSCReader *reader, QSmartCardData::PinType type, const QByteArray &puk, const QByteArray &pin) const final;
7173
bool updateCounters(QPCSCReader *reader, QSmartCardDataPrivate *d) const final;
7274

7375
static bool isSupported(const QByteArray &atr);
74-
static QByteArray pinTemplate(const QString &pin);
7576

7677
static const QByteArray AID;
7778
};

‎client/dialogs/PinPopup.cpp‎

Lines changed: 43 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -24,80 +24,80 @@
2424

2525
#include <QtCore/QTimeLine>
2626
#include <QtGui/QRegularExpressionValidator>
27-
#include <QtWidgets/QLineEdit>
2827
#include <QtWidgets/QProgressBar>
29-
#include <QtWidgets/QPushButton>
3028
#include <QSvgWidget>
3129

3230
PinPopup::PinPopup(QSmartCardData::PinType type, TokenFlags flags, const SslCertificate &c, QWidget *parent, QString text)
3331
: QDialog(parent)
34-
, ui(new Ui::PinPopup)
3532
{
36-
ui->setupUi(this);
33+
Ui::PinPopup ui {};
34+
ui.setupUi(this);
3735
setWindowFlags(Qt::Dialog | Qt::CustomizeWindowHint);
3836
#ifdef Q_OS_WIN
39-
ui->buttonLayout->setDirection(QBoxLayout::RightToLeft);
37+
ui.buttonLayout->setDirection(QBoxLayout::RightToLeft);
4038
#endif
4139
for(QLineEdit *w: findChildren<QLineEdit*>())
4240
w->setAttribute(Qt::WA_MacShowFocusRect, false);
43-
ui->pin->setValidator(regexp = new QRegularExpressionValidator(ui->pin));
41+
ui.pin->setValidator(regexp = new QRegularExpressionValidator(ui.pin));
4442
new Overlay(this);
4543

46-
connect( ui->ok, &QPushButton::clicked, this, &PinPopup::accept );
47-
connect( ui->cancel, &QPushButton::clicked, this, &PinPopup::reject );
48-
connect( this, &PinPopup::finished, this, &PinPopup::close );
49-
connect(ui->pin, &QLineEdit::returnPressed, ui->ok, &QPushButton::click);
44+
connect(ui.ok, &QPushButton::clicked, this, &PinPopup::accept);
45+
connect(ui.cancel, &QPushButton::clicked, this, &PinPopup::reject);
46+
connect(this, &PinPopup::finished, this, &PinPopup::close );
47+
connect(ui.pin, &QLineEdit::returnPressed, ui.ok, &QPushButton::click);
5048

5149
if(!text.isEmpty())
52-
ui->labelPin->hide();
50+
ui.labelPin->hide();
5351
else if(type == QSmartCardData::Pin2Type)
5452
{
5553
text = tr("Selected action requires sign certificate.");
56-
ui->labelPin->setText(flags & PinpadFlag ?
54+
ui.labelPin->setText(flags & PinpadFlag ?
5755
tr("For using sign certificate enter PIN2 at the reader") :
5856
tr("For using sign certificate enter PIN2"));
5957
setPinLen(5);
6058
}
6159
else
6260
{
6361
text = tr("Selected action requires authentication certificate.");
64-
ui->labelPin->setText(flags & PinpadFlag ?
62+
ui.labelPin->setText(flags & PinpadFlag ?
6563
tr("For using authentication certificate enter PIN1 at the reader") :
6664
tr("For using authentication certificate enter PIN1"));
6765
setPinLen(4);
6866
}
69-
ui->label->setText(c.toString(c.showCN() ? QStringLiteral("CN, serialNumber") : QStringLiteral("GN SN, serialNumber")));
70-
ui->text->setText(text);
67+
ui.label->setText(c.toString(c.showCN() ? QStringLiteral("CN, serialNumber") : QStringLiteral("GN SN, serialNumber")));
68+
ui.text->setText(text);
7169
if(flags & PinFinalTry)
72-
ui->errorPin->setText(tr("%1 will be locked next failed attempt").arg(QSmartCardData::typeString(type)));
70+
ui.errorPin->setText(tr("%1 will be locked next failed attempt").arg(QSmartCardData::typeString(type)));
7371
else if(flags & PinCountLow)
74-
ui->errorPin->setText(tr("%1 has been entered incorrectly at least once").arg(QSmartCardData::typeString(type)));
72+
ui.errorPin->setText(tr("%1 has been entered incorrectly at least once").arg(QSmartCardData::typeString(type)));
7573
else
76-
ui->errorPin->hide();
74+
ui.errorPin->hide();
7775

7876
if(flags & PinpadChangeFlag)
7977
{
80-
ui->pin->hide();
81-
ui->ok->hide();
82-
ui->cancel->hide();
83-
ui->errorPin->setAlignment(Qt::AlignCenter);
78+
isPinPad = true;
79+
ui.pin->hide();
80+
ui.ok->hide();
81+
ui.cancel->hide();
82+
ui.errorPin->setAlignment(Qt::AlignCenter);
8483
auto *movie = new QSvgWidget(QStringLiteral(":/images/wait.svg"), this);
85-
movie->setFixedSize(ui->pin->size().height(), ui->pin->size().height());
84+
movie->setFixedSize(ui.pin->size().height(), ui.pin->size().height());
8685
movie->show();
87-
ui->layoutContent->addWidget(movie, 0, Qt::AlignCenter);
86+
ui.layoutContent->addWidget(movie, 0, Qt::AlignCenter);
8887
}
8988
else if(flags & PinpadFlag)
9089
{
91-
ui->pin->hide();
92-
ui->ok->hide();
93-
ui->cancel->hide();
90+
isPinPad = true;
91+
ui.pin->hide();
92+
ui.ok->hide();
93+
ui.cancel->hide();
9494
auto *progress = new QProgressBar(this);
9595
progress->setRange( 0, 30 );
9696
progress->setValue( progress->maximum() );
9797
progress->setTextVisible( false );
9898
progress->resize( 200, 30 );
9999
progress->move( 153, 122 );
100-
ui->layoutContent->addWidget(progress);
100+
ui.layoutContent->addWidget(progress);
101101
auto *statusTimer = new QTimeLine(progress->maximum() * 1000, this);
102102
statusTimer->setEasingCurve(QEasingCurve::Linear);
103103
statusTimer->setFrameRange( progress->maximum(), progress->minimum() );
@@ -106,24 +106,20 @@ PinPopup::PinPopup(QSmartCardData::PinType type, TokenFlags flags, const SslCert
106106
}
107107
else
108108
{
109-
ui->pin->setFocus();
110-
ui->pin->setMaxLength( 12 );
111-
connect(ui->pin, &QLineEdit::textEdited, this, [&](const QString &text) {
112-
ui->ok->setEnabled(regexp->regularExpression().match(text).hasMatch());
109+
ui.pin->setFocus();
110+
ui.pin->setMaxLength( 12 );
111+
connect(ui.pin, &QLineEdit::textEdited, this, [&](const QString &text) {
112+
ui.ok->setEnabled(regexp->regularExpression().match(text).hasMatch());
113113
});
114-
ui->text->setBuddy( ui->pin );
115-
ui->ok->setDisabled(true);
114+
ui.text->setBuddy(ui.pin);
115+
ui.ok->setDisabled(true);
116116
}
117117
if(c.type() & SslCertificate::TempelType)
118118
{
119119
regexp->setRegularExpression(QRegularExpression(QStringLiteral("^.{4,}$")));
120-
ui->pin->setMaxLength(32767);
120+
ui.pin->setMaxLength(32767);
121121
}
122-
}
123-
124-
PinPopup::~PinPopup()
125-
{
126-
delete ui;
122+
pinInput = ui.pin;
127123
}
128124

129125
void PinPopup::setPinLen(unsigned long minLen, unsigned long maxLen)
@@ -132,4 +128,10 @@ void PinPopup::setPinLen(unsigned long minLen, unsigned long maxLen)
132128
regexp->setRegularExpression(QRegularExpression(QStringLiteral("^%1{%2,%3}$").arg(charPattern).arg(minLen).arg(maxLen)));
133129
}
134130

135-
QString PinPopup::pin() const { return ui->pin->text(); }
131+
QString PinPopup::pin() const { return pinInput->text(); }
132+
133+
void PinPopup::reject()
134+
{
135+
if (!isPinPad)
136+
QDialog::reject();
137+
}

‎client/dialogs/PinPopup.h‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,7 @@
2424
#include "QSmartCard.h"
2525
#include "WaitDialog.h"
2626

27-
namespace Ui {
28-
class PinPopup;
29-
}
30-
27+
class QLineEdit;
3128
class QRegularExpressionValidator;
3229
class SslCertificate;
3330

@@ -47,7 +44,6 @@ class PinPopup final : public QDialog
4744
Q_DECLARE_FLAGS(TokenFlags, TokenFlag)
4845

4946
PinPopup(QSmartCardData::PinType type, TokenFlags flags, const SslCertificate &cert, QWidget *parent = {}, QString text = {});
50-
~PinPopup() final;
5147

5248
void setPinLen(unsigned long minLen, unsigned long maxLen = 12);
5349
QString pin() const;
@@ -56,8 +52,11 @@ class PinPopup final : public QDialog
5652
void startTimer();
5753

5854
private:
59-
Ui::PinPopup *ui;
55+
void reject() final;
56+
57+
QLineEdit *pinInput;
6058
QRegularExpressionValidator *regexp {};
6159
WaitDialogHider hider;
60+
bool isPinPad = false;
6261
};
6362

0 commit comments

Comments
 (0)