Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix coverity warnings #68

Merged
merged 1 commit into from
Mar 1, 2024
Merged

Fix coverity warnings #68

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 30 additions & 5 deletions lib/libpcsc-cpp/include/pcsc-cpp/pcsc-cpp.hpp
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading