Skip to content

Commit

Permalink
Fix coverity warnings
Browse files Browse the repository at this point in the history 
IB-7930

Signed-off-by: Raul Metsma <[email protected]>
  • Loading branch information
@metsma @mrts
metsma authored and mrts committed Mar 1, 2024
1 parent a222d48 commit 6185adb
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 21 deletions.
35 changes: 30 additions & 5 deletions lib/libpcsc-cpp/include/pcsc-cpp/pcsc-cpp.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,25 @@
Class(Class&&) = delete; \
Class& operator=(Class&&) = delete

#ifdef WIN32
#define PCSC_CPP_WARNING_PUSH __pragma(warning(push))
#define PCSC_CPP_WARNING_POP __pragma(warning(pop))
#define PCSC_CPP_WARNING_DISABLE_CLANG(text)
#define PCSC_CPP_WARNING_DISABLE_GCC(text)
#define PCSC_CPP_WARNING_DISABLE_MSVC(number) __pragma(warning(disable: number))
#else
#define PCSC_CPP_DO_PRAGMA(text) _Pragma(#text)
#define PCSC_CPP_WARNING_PUSH PCSC_CPP_DO_PRAGMA(GCC diagnostic push)
#define PCSC_CPP_WARNING_POP PCSC_CPP_DO_PRAGMA(GCC diagnostic pop)
#if __clang__
#define PCSC_CPP_WARNING_DISABLE_CLANG(text) PCSC_CPP_DO_PRAGMA(clang diagnostic ignored text)
#else
#define PCSC_CPP_WARNING_DISABLE_CLANG(text)
#endif
#define PCSC_CPP_WARNING_DISABLE_GCC(text) PCSC_CPP_DO_PRAGMA(GCC diagnostic ignored text)
#define PCSC_CPP_WARNING_DISABLE_MSVC(text)
#endif

namespace pcsc_cpp
{

Expand Down Expand Up @@ -75,8 +94,8 @@ struct ResponseApdu

byte_vector data;

static const size_t MAX_DATA_SIZE = 256;
static const size_t MAX_SIZE = MAX_DATA_SIZE + 2; // + sw1 and sw2
static constexpr size_t MAX_DATA_SIZE = 256;
static constexpr size_t MAX_SIZE = MAX_DATA_SIZE + 2; // + sw1 and sw2

ResponseApdu(byte_type s1, byte_type s2, byte_vector d = {}) :
sw1(s1), sw2(s2), data(std::move(d))
Expand All @@ -85,15 +104,21 @@ struct ResponseApdu

ResponseApdu() = default;

static ResponseApdu fromBytes(const byte_vector& data)
static ResponseApdu fromBytes(byte_vector data)
{
if (data.size() < 2) {
throw std::invalid_argument("Need at least 2 bytes for creating ResponseApdu");
}

PCSC_CPP_WARNING_PUSH
PCSC_CPP_WARNING_DISABLE_GCC("-Warray-bounds") // avoid GCC 13 false positive warning
byte_type sw1 = data[data.size() - 2];
byte_type sw2 = data[data.size() - 1];
data.resize(data.size() - 2);
PCSC_CPP_WARNING_POP

// SW1 and SW2 are in the end
return ResponseApdu {data[data.size() - 2], data[data.size() - 1],
byte_vector {data.cbegin(), data.cend() - 2}};
return {sw1, sw2, std::move(data)};
}

byte_vector toBytes() const
Expand Down
8 changes: 4 additions & 4 deletions lib/libpcsc-cpp/src/SmartCard.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ class CardImpl
SCard(Transmit, cardHandle, &_protocol, commandBytes.data(), DWORD(commandBytes.size()),
nullptr, responseBytes.data(), &responseLength);

auto response = toResponse(responseBytes, responseLength);
auto response = toResponse(std::move(responseBytes), responseLength);

if (response.sw1 == ResponseApdu::MORE_DATA_AVAILABLE) {
getMoreResponseData(response);
Expand Down Expand Up @@ -175,7 +175,7 @@ class CardImpl
DWORD(responseBytes.size()), &responseLength);
}

return toResponse(responseBytes, responseLength);
return toResponse(std::move(responseBytes), responseLength);
}

void beginTransaction() const { SCard(BeginTransaction, cardHandle); }
Expand All @@ -189,7 +189,7 @@ class CardImpl
const SCARD_IO_REQUEST _protocol;
std::map<DRIVER_FEATURES, uint32_t> features;

ResponseApdu toResponse(byte_vector& responseBytes, size_t responseLength) const
ResponseApdu toResponse(byte_vector&& responseBytes, size_t responseLength) const
{
if (responseLength > responseBytes.size()) {
THROW(Error, "SCardTransmit: received more bytes than buffer size");
Expand All @@ -198,7 +198,7 @@ class CardImpl

// TODO: debug("Received: " + bytes2hexstr(responseBytes))

auto response = ResponseApdu::fromBytes(responseBytes);
auto response = ResponseApdu::fromBytes(std::move(responseBytes));

// Let expected errors through for handling in upper layers or in if blocks below.
switch (response.sw1) {
Expand Down
7 changes: 3 additions & 4 deletions src/electronic-id.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ constexpr auto constructor(const Reader& reader)
}

template <Pkcs11ElectronicIDType value>
constexpr auto constructor(const Reader&)
constexpr auto constructor(const Reader& /*reader*/)
{
return std::make_unique<Pkcs11ElectronicID>(value);
}
Expand Down Expand Up @@ -156,9 +156,8 @@ ElectronicID::ptr getElectronicID(const pcsc_cpp::Reader& reader)

bool ElectronicID::isSupportedSigningHashAlgorithm(const HashAlgorithm hashAlgo) const
{
auto supported = supportedSigningAlgorithms();
return std::any_of(supported.cbegin(), supported.cend(),
[hashAlgo](SignatureAlgorithm signAlgo) { return signAlgo == hashAlgo; });
const auto &supported = supportedSigningAlgorithms();
return std::find(supported.cbegin(), supported.cend(), hashAlgo) != supported.cend();
}

AutoSelectFailed::AutoSelectFailed(Reason r) :
Expand Down
2 changes: 1 addition & 1 deletion src/electronic-ids/pcsc/FinEID.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ byte_vector FinEIDv3::sign(const HashAlgorithm hashAlgo, const byte_vector& hash
byte_vector tlv {0x90, byte_type(hash.size())};
tlv.insert(tlv.cend(), hash.cbegin(), hash.cend());

const CommandApdu computeSignature {{0x00, 0x2A, 0x90, 0xA0}, tlv};
const CommandApdu computeSignature {{0x00, 0x2A, 0x90, 0xA0}, std::move(tlv)};
const auto response = card->transmit(computeSignature);

if (response.sw1 == ResponseApdu::WRONG_LENGTH) {
Expand Down
14 changes: 7 additions & 7 deletions tests/integration/test-authenticate.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,12 @@ TEST(electronic_id_test, authenticate)

EXPECT_TRUE(cardInfo);

std::cout << "Selected card: " << cardInfo->eid().name() << std::endl;
std::cout << "Selected card: " << cardInfo->eid().name() << '\n';

byte_vector cert = cardInfo->eid().getCertificate(CertificateType::AUTHENTICATION);

std::cout << "Does the reader have a PIN-pad? "
<< (cardInfo->eid().smartcard().readerHasPinPad() ? "yes" : "no") << std::endl;
<< (cardInfo->eid().smartcard().readerHasPinPad() ? "yes" : "no") << '\n';

if (cardInfo->eid().authSignatureAlgorithm() != JsonWebSignatureAlgorithm::ES384
&& cardInfo->eid().authSignatureAlgorithm() != JsonWebSignatureAlgorithm::RS256
Expand All @@ -52,21 +52,21 @@ TEST(electronic_id_test, authenticate)
"currently supported");
}

GTEST_ASSERT_GE(cardInfo->eid().authPinRetriesLeft().first, 0u);
GTEST_ASSERT_GE(cardInfo->eid().authPinRetriesLeft().first, 0U);

const auto pin = cardInfo->eid().name() == "EstEID Gemalto v3.5.8"
const auto &pin = cardInfo->eid().name() == "EstEID Gemalto v3.5.8"
? byte_vector {'0', '0', '9', '0'} // Gemalto test card default PIN1
: byte_vector {'1', '2', '3', '4'};

std::cout << "WARNING! Using hard-coded PIN "
<< std::string(reinterpret_cast<const char*>(pin.data()), pin.size()) << std::endl;
<< std::string(reinterpret_cast<const char*>(pin.data()), pin.size()) << '\n';

const byte_vector dataToSign = {'H', 'e', 'l', 'l', 'o', ' ', 'w', 'o', 'r', 'l', 'd', '!'};
const byte_vector dataToSign {'H', 'e', 'l', 'l', 'o', ' ', 'w', 'o', 'r', 'l', 'd', '!'};
const JsonWebSignatureAlgorithm hashAlgo = cardInfo->eid().authSignatureAlgorithm();
const byte_vector hash = calculateDigest(hashAlgo.hashAlgorithm(), dataToSign);
auto signature = cardInfo->eid().signWithAuthKey(pin, hash);

std::cout << "Authentication signature: " << pcsc_cpp::bytes2hexstr(signature) << std::endl;
std::cout << "Authentication signature: " << pcsc_cpp::bytes2hexstr(signature) << '\n';

if (!verify(hashAlgo.hashAlgorithm(), cert, dataToSign, signature,
hashAlgo == JsonWebSignatureAlgorithm::PS256)) {
Expand Down

0 comments on commit 6185adb

Please sign in to comment.