Skip to content

Commit 98115b1

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 98115b1

File tree

4 files changed

+110
-99
lines changed

4 files changed

+110
-99
lines changed

‎client/QSmartCard.cpp‎

Lines changed: 51 additions & 43 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;
@@ -119,6 +122,17 @@ QByteArrayView Card::parseFCI(QByteArrayView data, quint8 expectedTag)
119122
return {};
120123
}
121124

125+
QByteArray Card::pinTemplate(const QString &data) const
126+
{
127+
QByteArray pin = data.toUtf8();
128+
#if QT_VERSION >= QT_VERSION_CHECK(6, 4, 0)
129+
pin.resize(12, fillChar);
130+
#else
131+
pin += QByteArray(12 - pin.size(), fillChar);
132+
#endif
133+
return pin;
134+
}
135+
122136
struct TLV
123137
{
124138
quint32 tag {};
@@ -191,11 +205,9 @@ const QByteArray IDEMIACard::AID_QSCD = APDU("00A4040C 10 51534344204170706C6963
191205
const QByteArray IDEMIACard::ATR_COSMO8 = QByteArrayLiteral("3BDB960080B1FE451F830012233F536549440F9000F1");
192206
const QByteArray IDEMIACard::ATR_COSMOX = QByteArrayLiteral("3BDC960080B1FE451F830012233F54654944320F9000C3");
193207

194-
QPCSCReader::Result IDEMIACard::change(QPCSCReader *reader, QSmartCardData::PinType type, QByteArray &&pin, QByteArray &&newpin) const
208+
QPCSCReader::Result IDEMIACard::change(QPCSCReader *reader, QSmartCardData::PinType type, const QByteArray &pin, const QByteArray &newpin) const
195209
{
196210
QByteArray cmd = CHANGE;
197-
newpin = pinTemplate(std::move(newpin));
198-
pin = pinTemplate(std::move(pin));
199211
switch (type) {
200212
case QSmartCardData::Pin1Type:
201213
cmd[3] = 1;
@@ -210,7 +222,9 @@ QPCSCReader::Result IDEMIACard::change(QPCSCReader *reader, QSmartCardData::PinT
210222
break;
211223
}
212224
cmd[4] = char(pin.size() + newpin.size());
213-
return transfer(reader, false, cmd + pin + newpin, type, quint8(pin.size()), true);
225+
cmd += pin;
226+
cmd += newpin;
227+
return transfer(reader, false, std::move(cmd), type, quint8(pin.size()), true);
214228
}
215229

216230
bool IDEMIACard::isSupported(const QByteArray &atr)
@@ -291,23 +305,13 @@ bool IDEMIACard::loadPerso(QPCSCReader *reader, QSmartCardDataPrivate *d) const
291305
return updateCounters(reader, d);
292306
}
293307

294-
QByteArray IDEMIACard::pinTemplate(QByteArray &&pin)
295-
{
296-
#if QT_VERSION >= QT_VERSION_CHECK(6, 4, 0)
297-
pin.resize(12, char(0xFF));
298-
#else
299-
pin += QByteArray(12 - pin.size(), char(0xFF));
300-
#endif
301-
return std::move(pin);
302-
}
303-
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,14 @@ bool THALESCard::loadPerso(QPCSCReader *reader, QSmartCardDataPrivate *d) const
441445
return updateCounters(reader, d);
442446
}
443447

444-
QByteArray THALESCard::pinTemplate(const QString &pin)
445-
{
446-
QByteArray result = pin.toUtf8();
447-
#if QT_VERSION >= QT_VERSION_CHECK(6, 4, 0)
448-
result.resize(12, char(0x00));
449-
#else
450-
result += QByteArray(12 - result.size(), char(0x00));
451-
#endif
452-
return result;
453-
}
454-
455-
QPCSCReader::Result THALESCard::replace(QPCSCReader *reader, QSmartCardData::PinType type, QByteArray &&puk, QByteArray &&pin) const
448+
QPCSCReader::Result THALESCard::replace(QPCSCReader *reader, QSmartCardData::PinType type, const QByteArray &puk, const QByteArray &pin) const
456449
{
457-
puk = pinTemplate(std::move(puk));
458-
pin = pinTemplate(std::move(pin));
459450
QByteArray cmd = REPLACE;
460451
cmd[3] = char(0x80 | type);
461452
cmd[4] = char(puk.size() + pin.size());
462-
return transfer(reader, false, cmd + puk + pin, type, quint8(puk.size()), true);
453+
cmd += puk;
454+
cmd += pin;
455+
return transfer(reader, false, std::move(cmd), type, quint8(puk.size()), true);
463456
}
464457

465458
bool THALESCard::updateCounters(QPCSCReader *reader, QSmartCardDataPrivate *d) const
@@ -533,16 +526,31 @@ bool QSmartCard::pinChange(QSmartCardData::PinType type, QSmartCard::PinAction a
533526
}
534527
popup.reset(new PinPopup(src, flags, d->t.authCert(), parent, bodyText));
535528
popup->open();
529+
oldPin = d->card->pinTemplate({});
530+
newPin = d->card->pinTemplate({});
536531
}
537532
else
538533
{
539534
PinUnblock p(type, action, d->t.retryCount(src), d->t.data(QSmartCardData::BirthDate).toDate(),
540535
d->t.data(QSmartCardData::Id).toString(), d->t.isPUKReplacable(), parent);
541536
if (!p.exec())
542537
return false;
543-
oldPin = p.firstCodeText().toUtf8();
544-
newPin = p.newCodeText().toUtf8();
538+
QString oldPinString = p.firstCodeText();
539+
QString newPinString = p.newCodeText();
540+
oldPin = d->card->pinTemplate(oldPinString);
541+
newPin = d->card->pinTemplate(newPinString);
542+
// Try to clean QLineEdit internal PIN copy using constData that does not detach memory
543+
auto chars = const_cast<QChar*>(oldPinString.constData());
544+
for (int i = 0; i < oldPinString.length(); ++i)
545+
chars[i] = '\0';
546+
chars = const_cast<QChar*>(newPinString.constData());
547+
for (int i = 0; i < newPinString.length(); ++i)
548+
chars[i] = '\0';
545549
}
550+
auto clean = qScopeGuard([&oldPin, &newPin] {
551+
oldPin.fill('\0');
552+
newPin.fill('\0');
553+
});
546554

547555
QPCSCReader reader(d->t.reader(), &QPCSC::instance());
548556
if(!reader.connect())
@@ -551,8 +559,8 @@ bool QSmartCard::pinChange(QSmartCardData::PinType type, QSmartCard::PinAction a
551559
return false;
552560
}
553561
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));
562+
d->card->change(&reader, type, oldPin, newPin) :
563+
d->card->replace(&reader, type, oldPin, newPin);
556564
switch(response.SW)
557565
{
558566
case 0x9000:

‎client/QSmartCard_p.h‎

Lines changed: 11 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;
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

@@ -46,32 +47,33 @@ class Card
4647
static const QByteArray READBINARY;
4748
static const QByteArray REPLACE;
4849
static const QByteArray VERIFY;
50+
51+
char fillChar = 0x00;
4952
};
5053

5154
class IDEMIACard: public Card
5255
{
5356
public:
54-
QPCSCReader::Result change(QPCSCReader *reader, QSmartCardData::PinType type, QByteArray &&pin, QByteArray &&newpin) const final;
57+
IDEMIACard() { fillChar = 0xFF; }
58+
QPCSCReader::Result change(QPCSCReader *reader, QSmartCardData::PinType type, const QByteArray &pin, const QByteArray &newpin) const final;
5559
bool loadPerso(QPCSCReader *reader, QSmartCardDataPrivate *d) const final;
56-
QPCSCReader::Result replace(QPCSCReader *reader, QSmartCardData::PinType type, QByteArray &&puk, QByteArray &&pin) const final;
60+
QPCSCReader::Result replace(QPCSCReader *reader, QSmartCardData::PinType type, const QByteArray &puk, const QByteArray &pin) const final;
5761
bool updateCounters(QPCSCReader *reader, QSmartCardDataPrivate *d) const final;
5862

5963
static bool isSupported(const QByteArray &atr);
60-
static QByteArray pinTemplate(QByteArray &&pin);
6164

6265
static const QByteArray AID, AID_OT, AID_QSCD, ATR_COSMO8, ATR_COSMOX;
6366
};
6467

6568
class THALESCard: public Card
6669
{
6770
public:
68-
QPCSCReader::Result change(QPCSCReader *reader, QSmartCardData::PinType type, QByteArray &&pin, QByteArray &&newpin) const final;
71+
QPCSCReader::Result change(QPCSCReader *reader, QSmartCardData::PinType type, const QByteArray &pin, const QByteArray &newpin) const final;
6972
bool loadPerso(QPCSCReader *reader, QSmartCardDataPrivate *d) const final;
70-
QPCSCReader::Result replace(QPCSCReader *reader, QSmartCardData::PinType type, QByteArray &&puk, QByteArray &&pin) const final;
73+
QPCSCReader::Result replace(QPCSCReader *reader, QSmartCardData::PinType type, const QByteArray &puk, const QByteArray &pin) const final;
7174
bool updateCounters(QPCSCReader *reader, QSmartCardDataPrivate *d) const final;
7275

7376
static bool isSupported(const QByteArray &atr);
74-
static QByteArray pinTemplate(const QString &pin);
7577

7678
static const QByteArray AID;
7779
};

‎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)