From dce9231b3efbb887b2ab164f1c11227e8042f77a Mon Sep 17 00:00:00 2001 From: Raul Metsma Date: Mon, 17 Aug 2026 13:15:15 +0300 Subject: [PATCH] Harden Smart Card property handling IB-9050 Signed-off-by: Raul Metsma --- client/QPCSC.cpp | 63 +++++++++++++++++++++++-------------------- client/QPCSC.h | 1 - client/QPCSC_p.h | 4 +-- client/QSmartCard.cpp | 12 ++++----- 4 files changed, 42 insertions(+), 38 deletions(-) diff --git a/client/QPCSC.cpp b/client/QPCSC.cpp index 3999ecd5e..42aa83445 100644 --- a/client/QPCSC.cpp +++ b/client/QPCSC.cpp @@ -63,22 +63,25 @@ static auto SCCall(const char *file, int line, const char *function, Args&& ...a } #define SC(API, ...) SCCall(__FILE__, __LINE__, "SCard"#API, __VA_ARGS__) -QHash QPCSCReader::Private::features() +quint32 QPCSCReader::Private::feature(DRIVER_FEATURES key) { - if(!featuresList.isEmpty()) - return featuresList; - DWORD size = 0; - std::array feature{}; - if(SC(Control, card, DWORD(CM_IOCTL_GET_FEATURE_REQUEST), nullptr, 0U, feature.data(), DWORD(feature.size() * sizeof(PCSC_TLV_STRUCTURE)), &size)) - return featuresList; - if(size % sizeof(PCSC_TLV_STRUCTURE)) - return featuresList; - for(const auto &f: feature) + if(features[0].tag == 0) + { + if(DWORD size = 0, capacity = DWORD(features.size() * sizeof(PCSC_TLV_STRUCTURE)); + SC(Control, card, DWORD(CM_IOCTL_GET_FEATURE_REQUEST), nullptr, 0U, features.data(), capacity, &size) || + size > capacity || + size % sizeof(PCSC_TLV_STRUCTURE) != 0) + { + features.fill(PCSC_TLV_STRUCTURE{}); + return 0; + } + } + for(const auto &f: features) { - if(f.tag) - featuresList[f.tag] = qFromBigEndian(f.value); + if(f.tag == key) + return qFromBigEndian(f.value); } - return featuresList; + return 0; } @@ -177,7 +180,6 @@ void QPCSC::run() Q_EMIT cardChanged(); i->dwCurrentState = i->dwEventState; qCDebug(SCard) << "New state: " << QString::fromLocal8Bit(i->szReader) << stateToString(i->dwCurrentState); - Q_EMIT statusChanged(QString::fromLocal8Bit(i->szReader), stateToString(i->dwCurrentState)); if((i->dwCurrentState & (SCARD_STATE_UNKNOWN|SCARD_STATE_IGNORE)) > 0) { free((void*)i->szReader); @@ -245,8 +247,7 @@ bool QPCSCReader::isPinPad() const return false; if(qEnvironmentVariableIsSet("SMARTCARDPP_NOPINPAD")) return false; - QHash features = d->features(); - return features.contains(FEATURE_VERIFY_PIN_DIRECT) || features.contains(FEATURE_VERIFY_PIN_START); + return d->feature(FEATURE_VERIFY_PIN_DIRECT) || d->feature(FEATURE_VERIFY_PIN_START); } bool QPCSCReader::isPresent() const @@ -262,18 +263,23 @@ QString QPCSCReader::name() const QHash QPCSCReader::properties() const { QHash properties; - if( DWORD ioctl = d->features().value(FEATURE_GET_TLV_PROPERTIES) ) + if(DWORD ioctl = d->feature(FEATURE_GET_TLV_PROPERTIES)) { DWORD size = 0; std::array recv{}; - if(SC(Control, d->card, ioctl, nullptr, 0U, recv.data(), DWORD(recv.size()), &size)) + if(SC(Control, d->card, ioctl, nullptr, 0U, recv.data(), DWORD(recv.size()), &size) || size > recv.size()) return properties; - for(auto p = recv.cbegin(); std::distance(recv.cbegin(), p) < size; ) + auto end = std::next(recv.cbegin(), size); + for(auto p = recv.cbegin(); std::distance(p, end) >= 2; ) { - int tag = *p++, len = *p++, value = 0; - for(int i = 0; i < len; ++i) - value |= *p++ << 8 * i; - properties.emplace(Properties(tag), value); + auto tag = Properties(*p++); + size_t len = *p++; + if(len > size_t(std::distance(p, end)) || len > sizeof(quint32)) + return properties; + quint32 value = 0; + for(size_t i = 0; i < len; ++i) + value |= quint32(*p++) << (8 * i); + properties.emplace(tag, int(value)); } } return properties; @@ -323,8 +329,7 @@ QPCSCReader::Result QPCSCReader::transferCTL(const QByteArray &apdu, bool verify quint16 lang, quint8 minlen, quint8 newPINOffset, bool requestCurrentPIN) const { bool display = false; - QHash features = d->features(); - if( DWORD ioctl = features.value(FEATURE_IFD_PIN_PROPERTIES) ) + if(DWORD ioctl = d->feature(FEATURE_IFD_PIN_PROPERTIES)) { PIN_PROPERTIES_STRUCTURE caps{}; DWORD size = sizeof(caps); @@ -377,9 +382,9 @@ QPCSCReader::Result QPCSCReader::transferCTL(const QByteArray &apdu, bool verify cmd = toByteArray(data); } - DWORD ioctl = features.value( verify ? FEATURE_VERIFY_PIN_START : FEATURE_MODIFY_PIN_START ); - if( !ioctl ) - ioctl = features.value( verify ? FEATURE_VERIFY_PIN_DIRECT : FEATURE_MODIFY_PIN_DIRECT ); + DWORD ioctl = d->feature(verify ? FEATURE_VERIFY_PIN_START : FEATURE_MODIFY_PIN_START); + if(!ioctl) + ioctl = d->feature(verify ? FEATURE_VERIFY_PIN_DIRECT : FEATURE_MODIFY_PIN_DIRECT); qCDebug(APDU).nospace().noquote() << 'T' << d->io.dwProtocol - 1 << "> " << apdu.toHex(); qCDebug(APDU).nospace().noquote() << "CTL" << "> " << cmd.toHex(); @@ -387,7 +392,7 @@ QPCSCReader::Result QPCSCReader::transferCTL(const QByteArray &apdu, bool verify auto size = DWORD(data.size()); LONG err = SC(Control, d->card, ioctl, cmd.constData(), DWORD(cmd.size()), LPVOID(data.data()), DWORD(data.size()), &size); - if( DWORD finish = features.value( verify ? FEATURE_VERIFY_PIN_FINISH : FEATURE_MODIFY_PIN_FINISH ) ) + if(DWORD finish = d->feature(verify ? FEATURE_VERIFY_PIN_FINISH : FEATURE_MODIFY_PIN_FINISH)) { size = DWORD(data.size()); err = SC(Control, d->card, finish, nullptr, 0U, LPVOID(data.data()), DWORD(data.size()), &size); diff --git a/client/QPCSC.h b/client/QPCSC.h index 5e313b30a..aa7397bbf 100644 --- a/client/QPCSC.h +++ b/client/QPCSC.h @@ -35,7 +35,6 @@ class QPCSC final: public QThread bool serviceRunning() const; Q_SIGNALS: - void statusChanged(const QString &reader, const QStringList &state); void cardChanged(); private: diff --git a/client/QPCSC_p.h b/client/QPCSC_p.h index 7fd830fe8..d6da4d33f 100644 --- a/client/QPCSC_p.h +++ b/client/QPCSC_p.h @@ -189,7 +189,7 @@ struct QPCSC::Private struct QPCSCReader::Private { - QHash features(); + quint32 feature(DRIVER_FEATURES key); QPCSC::Private *d {}; SCARDHANDLE card {}; @@ -198,5 +198,5 @@ struct QPCSCReader::Private QByteArray reader; bool isTransacted {}; - QHash featuresList; + std::array features{}; }; diff --git a/client/QSmartCard.cpp b/client/QSmartCard.cpp index 764cc0ef4..46c9b5f8f 100644 --- a/client/QSmartCard.cpp +++ b/client/QSmartCard.cpp @@ -122,9 +122,11 @@ QPCSCReader::Result Card::transfer(bool verify, QByteArray &&apdu, QByteArrayView Card::parseFCI(QByteArrayView data, quint8 expectedTag) { - for(auto i = data.begin(); i != data.end();) + for(auto i = data.begin(); std::distance(i, data.end()) >= 2;) { quint8 tag(*i++), size(*i++); + if(size > std::distance(i, data.end())) + return {}; if(tag == expectedTag) return {i, size}; if((tag & 0x20) == 0) @@ -491,7 +493,7 @@ bool THALESCard::updateCounters(QSmartCardDataPrivate *d) const QSmartCard::QSmartCard(QObject *parent) : QObject(parent) - , d(new Private) + , d(std::make_unique()) {} QSmartCard::~QSmartCard() noexcept = default; @@ -658,7 +660,7 @@ void QSmartCard::reloadCard(const TokenData &token, bool reloadCounters) QString reader = d->token.reader(); if(d->token.reader().endsWith(QLatin1String("..."))) { - for(auto truncated = QStringView(d->token.reader()).left(d->token.reader().size() - 3); + for(auto truncated = QStringView(reader).left(reader.size() - 3); const QString &test: QPCSC::instance().readers()) { if(test.startsWith(truncated)) reader = test; @@ -667,9 +669,7 @@ void QSmartCard::reloadCard(const TokenData &token, bool reloadCounters) qCDebug(CLog) << "Read" << reader; auto card = Card::card(reader); - if(!card) - return; - if(!card->reader.connect()) + if(!card || !card->reader.connect()) return; qCDebug(CLog) << "Read card" << d->token.card() << "info"; QSharedDataPointer t = d->t.d;