Skip to content

Commit e7a7469

Browse files
metsmametsma
committed
Do not allow cancel in PinPad view and clean PIN memory
IB-7082 Signed-off-by: Raul Metsma <raul@metsma.ee>
1 parent e82f2a6 commit e7a7469

4 files changed

Lines changed: 106 additions & 85 deletions

File tree

‎client/QSmartCard.cpp‎

Lines changed: 48 additions & 29 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;
@@ -210,7 +211,9 @@ QPCSCReader::Result IDEMIACard::change(QPCSCReader *reader, QSmartCardData::PinT
210211
break;
211212
}
212213
cmd[4] = char(pin.size() + newpin.size());
213-
return transfer(reader, false, cmd + pin + newpin, type, quint8(pin.size()), true);
214+
cmd += pin;
215+
cmd += newpin;
216+
return transfer(reader, false, std::move(cmd), type, quint8(pin.size()), true);
214217
}
215218

216219
bool IDEMIACard::isSupported(const QByteArray &atr)
@@ -291,23 +294,24 @@ bool IDEMIACard::loadPerso(QPCSCReader *reader, QSmartCardDataPrivate *d) const
291294
return updateCounters(reader, d);
292295
}
293296

294-
QByteArray IDEMIACard::pinTemplate(QByteArray &&pin)
297+
QByteArray IDEMIACard::pinTemplate(const QString &data) const
295298
{
299+
QByteArray pin = data.toUtf8();
296300
#if QT_VERSION >= QT_VERSION_CHECK(6, 4, 0)
297301
pin.resize(12, char(0xFF));
298302
#else
299303
pin += QByteArray(12 - pin.size(), char(0xFF));
300304
#endif
301-
return std::move(pin);
305+
return pin;
302306
}
303307

304-
QPCSCReader::Result IDEMIACard::replace(QPCSCReader *reader, QSmartCardData::PinType type, QByteArray &&puk, QByteArray &&pin) const
308+
QPCSCReader::Result IDEMIACard::replace(QPCSCReader *reader, QSmartCardData::PinType type, const QByteArray &puk, const QByteArray &pin) const
305309
{
306-
puk = pinTemplate(std::move(puk));
307310
QByteArray cmd = VERIFY;
308311
cmd[3] = 2;
309312
cmd[4] = char(puk.size());
310-
if(auto result = transfer(reader, true, cmd + puk, QSmartCardData::PukType, 0, true); !result)
313+
cmd += puk;
314+
if(auto result = transfer(reader, true, std::move(cmd), QSmartCardData::PukType, 0, true); !result)
311315
return result;
312316

313317
cmd = Card::REPLACE;
@@ -320,9 +324,9 @@ QPCSCReader::Result IDEMIACard::replace(QPCSCReader *reader, QSmartCardData::Pin
320324
}
321325
else
322326
cmd[3] = char(type);
323-
pin = pinTemplate(std::move(pin));
324327
cmd[4] = char(pin.size());
325-
return transfer(reader, false, cmd + pin, type, 0, false);
328+
cmd += pin;
329+
return transfer(reader, false, std::move(cmd), type, 0, false);
326330
}
327331

328332
bool IDEMIACard::updateCounters(QPCSCReader *reader, QSmartCardDataPrivate *d) const
@@ -351,14 +355,14 @@ bool IDEMIACard::updateCounters(QPCSCReader *reader, QSmartCardDataPrivate *d) c
351355

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

354-
QPCSCReader::Result THALESCard::change(QPCSCReader *reader, QSmartCardData::PinType type, QByteArray &&pin, QByteArray &&newpin) const
358+
QPCSCReader::Result THALESCard::change(QPCSCReader *reader, QSmartCardData::PinType type, const QByteArray &pin, const QByteArray &newpin) const
355359
{
356360
QByteArray cmd = CHANGE;
357-
newpin = pinTemplate(std::move(newpin));
358-
pin = pinTemplate(std::move(pin));
359361
cmd[3] = char(0x80 | type);
360362
cmd[4] = char(pin.size() + newpin.size());
361-
return transfer(reader, false, cmd + pin + newpin, type, quint8(pin.size()), true);
363+
cmd += pin;
364+
cmd += newpin;
365+
return transfer(reader, false, std::move(cmd), type, quint8(pin.size()), true);
362366
}
363367

364368
bool THALESCard::isSupported(const QByteArray &atr)
@@ -441,25 +445,25 @@ bool THALESCard::loadPerso(QPCSCReader *reader, QSmartCardDataPrivate *d) const
441445
return updateCounters(reader, d);
442446
}
443447

444-
QByteArray THALESCard::pinTemplate(const QString &pin)
448+
QByteArray THALESCard::pinTemplate(const QString &data) const
445449
{
446-
QByteArray result = pin.toUtf8();
450+
QByteArray pin = data.toUtf8();
447451
#if QT_VERSION >= QT_VERSION_CHECK(6, 4, 0)
448-
result.resize(12, char(0x00));
452+
pin.resize(12, char(0x00));
449453
#else
450-
result += QByteArray(12 - result.size(), char(0x00));
454+
pin += QByteArray(12 - result.size(), char(0x00));
451455
#endif
452-
return result;
456+
return pin;
453457
}
454458

455-
QPCSCReader::Result THALESCard::replace(QPCSCReader *reader, QSmartCardData::PinType type, QByteArray &&puk, QByteArray &&pin) const
459+
QPCSCReader::Result THALESCard::replace(QPCSCReader *reader, QSmartCardData::PinType type, const QByteArray &puk, const QByteArray &pin) const
456460
{
457-
puk = pinTemplate(std::move(puk));
458-
pin = pinTemplate(std::move(pin));
459461
QByteArray cmd = REPLACE;
460462
cmd[3] = char(0x80 | type);
461463
cmd[4] = char(puk.size() + pin.size());
462-
return transfer(reader, false, cmd + puk + pin, type, quint8(puk.size()), true);
464+
cmd += puk;
465+
cmd += pin;
466+
return transfer(reader, false, std::move(cmd), type, quint8(puk.size()), true);
463467
}
464468

465469
bool THALESCard::updateCounters(QPCSCReader *reader, QSmartCardDataPrivate *d) const
@@ -533,16 +537,31 @@ bool QSmartCard::pinChange(QSmartCardData::PinType type, QSmartCard::PinAction a
533537
}
534538
popup.reset(new PinPopup(src, flags, d->t.authCert(), parent, bodyText));
535539
popup->open();
540+
oldPin = d->card->pinTemplate({});
541+
newPin = d->card->pinTemplate({});
536542
}
537543
else
538544
{
539545
PinUnblock p(type, action, d->t.retryCount(src), d->t.data(QSmartCardData::BirthDate).toDate(),
540546
d->t.data(QSmartCardData::Id).toString(), d->t.isPUKReplacable(), parent);
541547
if (!p.exec())
542548
return false;
543-
oldPin = p.firstCodeText().toUtf8();
544-
newPin = p.newCodeText().toUtf8();
549+
QString oldPinString = p.firstCodeText();
550+
QString newPinString = p.newCodeText();
551+
oldPin = d->card->pinTemplate(oldPinString);
552+
newPin = d->card->pinTemplate(newPinString);
553+
// Try to clean QLineEdit internal PIN copy using constData that does not detach memory
554+
auto chars = const_cast<QChar*>(oldPinString.constData());
555+
for (int i = 0; i < oldPinString.length(); ++i)
556+
chars[i] = '\0';
557+
chars = const_cast<QChar*>(newPinString.constData());
558+
for (int i = 0; i < newPinString.length(); ++i)
559+
chars[i] = '\0';
545560
}
561+
auto clean = qScopeGuard([&oldPin, &newPin] {
562+
oldPin.fill('\0');
563+
newPin.fill('\0');
564+
});
546565

547566
QPCSCReader reader(d->t.reader(), &QPCSC::instance());
548567
if(!reader.connect())
@@ -551,8 +570,8 @@ bool QSmartCard::pinChange(QSmartCardData::PinType type, QSmartCard::PinAction a
551570
return false;
552571
}
553572
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));
573+
d->card->change(&reader, type, oldPin, newPin) :
574+
d->card->replace(&reader, type, oldPin, newPin);
556575
switch(response.SW)
557576
{
558577
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+
}

0 commit comments

Comments
 (0)