Memory leak in signature verification

[zzandyc]

I use the following code for signature verification, and it seems that there is a memory leak that occurred in the function RSA_VerifyStr,inside the StringSource,
I don't know what caused it, I hope to receive a response.
cryptopp version is 8.6.0,operating system is win10,IDE is vs2022

`#include
#include <cryptopp/rsa.h>
#include <cryptopp/randpool.h>
#include <cryptopp/osrng.h>
#include <cryptopp/files.h>
#include <cryptopp/base64.h>
#include <cryptopp/aes.h>
#include <cryptopp/hex.h>
#include <cryptopp/modes.h>
#include <cryptopp/sha.h>

using namespace CryptoPP;

std::string SHA256EncodeStr(const std::string& plainText)
{
SHA256 sha256;
std::string hash;
StringSource ss(plainText, true, new HashFilter(sha256, new HexEncoder(new StringSink(hash))));
return hash;
}

bool RSA_VerifyStr(const std::string& pubStr, const std::string& message, const std::string& signatureStr)
{
StringSource pub(pubStr.c_str(), true, new HexDecoder);
RSASS<PKCS1v15, SHA1>::Verifier pubVerifier(pub);

StringSource signatureSrc(signatureStr.c_str(), true, new HexDecoder);
if (signatureSrc.MaxRetrievable() != pubVerifier.SignatureLength())
    return false;

SecByteBlock signature(pubVerifier.SignatureLength());
signatureSrc.Get(signature, signature.size());

SignatureVerificationFilter* verifierFilter = new SignatureVerificationFilter(pubVerifier);
verifierFilter->Put(signature, pubVerifier.SignatureLength());
StringSource s((byte*)message.c_str(), message.length(), true, verifierFilter);

return verifierFilter->GetLastResult();

}

int main()
{
std::string pubKeyStr = "30819D300D06092A864886F70D010101050003818B00308187028181009CE8D41CF3B62F8CDBA9B020D9D4A4CFEE9CDF0A49FBA990D2EFD1160649197D206B3D47AC52B6B982E3936EDCCFC850EFF5FEF32B7E7DBB0C017B56CF0FD4FC20ECF8DD58D232569CFAD1AF25DE1CCAABDD85153B572B96A241C49D6E6DBBFC19DB1CEE444488606D6CE0A27E214408FCF727923AEB641E0EF922368582001B020111";
std::string signature = "06B32FEF7F4A5EB12F809F641A7E8F84465401CD212B6B775BA658855C0CD8B417D54D3FDC8DD64FDDD2B04C14A94B5C37EC5C8A2748F97332EF251C02D2958CC88CA3E4A144DD04F609B0BD08043B6FD90E9C369214C84F24A374883CDF836B11156378EF05E9C1E8677090EFD6362A006B4ADD29F1CDDF9D26FFD621D2A7EB";
std::string jsonStr = "{"validFrom":1703001600,"validTo":1734624000,"cpuId":"BFEBFBFF000A0652"}";
if (!RSA_VerifyStr(pubKeyStr, SHA256EncodeStr(jsonStr), signature))
return -1;

return 0;

}
`

[dangdkhanh]

same problem.

[xamelllion]

Valgrind shows that there is some still reachable block of memory, but it doesn't look like some big problem.

==109758== Memcheck, a memory error detector
==109758== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==109758== Using Valgrind-3.21.0 and LibVEX; rerun with -h for copyright info
==109758== Command: ./a.out
==109758== 
==109758== 
==109758== HEAP SUMMARY:
==109758==     in use at exit: 8 bytes in 1 blocks
==109758==   total heap usage: 161 allocs, 160 frees, 92,881 bytes allocated
==109758== 
==109758== LEAK SUMMARY:
==109758==    definitely lost: 0 bytes in 0 blocks
==109758==    indirectly lost: 0 bytes in 0 blocks
==109758==      possibly lost: 0 bytes in 0 blocks
==109758==    still reachable: 8 bytes in 1 blocks
==109758==         suppressed: 0 bytes in 0 blocks
==109758== Reachable blocks (those to which a pointer was found) are not shown.
==109758== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==109758== 
==109758== For lists of detected and suppressed errors, rerun with: -s
==109758== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

[zzandyc]

But if this code is run in a thread loop, memory leak is a big problem.

[EduardoMiravalls]

This is a false positive that you will usually find in code using global singletons, and it is actually documented in the Singleton class to "(...) avoid a subtle initialization problem in a multi-threaded environment with thread local storage on early Windows platforms (...)".

If you look at misc.h, you will see that the variable s_pObject in line 333 is an atomic singleton that it is never freed.
If you run your mwe with valgrind --leak-check=full --show-leak-kinds=all --track-origins=yes you will see something like this:

==13688== 8 bytes in 1 blocks are still reachable in loss record 1 of 1
==13688==    at 0x4849013: operator new(unsigned long) (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==13688==    by 0x177EC0: CryptoPP::NewObject<CryptoPP::PKCS1v15_SignatureMessageEncodingMethod>::operator()() const (misc.h:260)
==13688==    by 0x17798A: CryptoPP::Singleton<CryptoPP::PKCS1v15_SignatureMessageEncodingMethod, CryptoPP::NewObject<CryptoPP::PKCS1v15_SignatureMessageEncodingMethod>, 0>::Ref() const (misc.h:348)
==13688==    by 0x17647A: CryptoPP::TF_ObjectImplBase<CryptoPP::TF_VerifierBase, CryptoPP::TF_SignatureSchemeOptions<CryptoPP::TF_SS<CryptoPP::RSA, CryptoPP::PKCS1v15, CryptoPP::SHA1, int>, CryptoPP::RSA, CryptoPP::PKCS1v15_SignatureMessageEncodingMethod, CryptoPP::SHA1>, CryptoPP::RSAFunction>::GetMessageEncodingInterface() const (pubkey.h:594)
==13688==    by 0x232C46: CryptoPP::TF_VerifierBase::InputSignature(CryptoPP::PK_MessageAccumulator&, unsigned char const*, unsigned long) const (pubkey.cpp:98)
==13688==    by 0x1FC6F5: CryptoPP::SignatureVerificationFilter::LastPut(unsigned char const*, unsigned long) (filters.cpp:1139)
==13688==    by 0x1FF32F: CryptoPP::FilterWithBufferedInput::PutMaybeModifiable(unsigned char*, unsigned long, int, bool, bool) (filters.cpp:427)
==13688==    by 0x1712BC: CryptoPP::FilterWithBufferedInput::Put2(unsigned char const*, unsigned long, int, bool) (filters.h:361)
==13688==    by 0x1795EB: CryptoPP::BufferedTransformation::ChannelPut2(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, unsigned char const*, unsigned long, int, bool) (cryptlib.cpp:469)
==13688==    by 0x17A6DD: ChannelMessageEnd (cryptlib.h:2258)
==13688==    by 0x17A6DD: CryptoPP::BufferedTransformation::TransferMessagesTo2(CryptoPP::BufferedTransformation&, unsigned int&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, bool) (cryptlib.cpp:661)
==13688==    by 0x17B163: CryptoPP::BufferedTransformation::TransferAllTo2(CryptoPP::BufferedTransformation&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, bool) (cryptlib.cpp:702)
==13688==    by 0x1770BF: CryptoPP::SourceTemplate<CryptoPP::StringStore>::PumpAll2(bool) (filters.h:1444)

What valgrind is reporting is that the variable is allocated once in line 348 or 361 (depending on compilation flags) and never explicitly freed.
If we try instead calling RSA_VerifyStr in a loop, for example 10 times:

for (int i = 0; i < 10; i++) {
	if (!RSA_VerifyStr(pubKeyStr, SHA256EncodeStr(jsonStr), signature)) {
		return -1;
	}
}

there are more allocations (as expected), but the number of blocks still reachable reported by valgrind is still 1:

==17861==     in use at exit: 8 bytes in 1 blocks
==17861==   total heap usage: 1,521 allocs, 1,520 frees, 258,791 bytes allocated
==17861==
==17861== 8 bytes in 1 blocks are still reachable in loss record 1 of 1
==17861==    at 0x4849013: operator new(unsigned long) (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==17861==    by 0x177EDC: CryptoPP::NewObject<CryptoPP::PKCS1v15_SignatureMessageEncodingMethod>::operator()() const (misc.h:260)
==17861==    by 0x1779A6: CryptoPP::Singleton<CryptoPP::PKCS1v15_SignatureMessageEncodingMethod, CryptoPP::NewObject<CryptoPP::PKCS1v15_SignatureMessageEncodingMethod>, 0>::Ref() const (misc.h:348)
==17861==    by 0x176496: CryptoPP::TF_ObjectImplBase<CryptoPP::TF_VerifierBase, CryptoPP::TF_SignatureSchemeOptions<CryptoPP::TF_SS<CryptoPP::RSA, CryptoPP::PKCS1v15, CryptoPP::SHA1, int>, CryptoPP::RSA, CryptoPP::PKCS1v15_SignatureMessageEncodingMethod, CryptoPP::SHA1>, CryptoPP::RSAFunction>::GetMessageEncodingInterface() const (pubkey.h:594)
==17861==    by 0x232C66: CryptoPP::TF_VerifierBase::InputSignature(CryptoPP::PK_MessageAccumulator&, unsigned char const*, unsigned long) const (pubkey.cpp:98)
==17861==    by 0x1FC715: CryptoPP::SignatureVerificationFilter::LastPut(unsigned char const*, unsigned long) (filters.cpp:1139)
==17861==    by 0x1FF34F: CryptoPP::FilterWithBufferedInput::PutMaybeModifiable(unsigned char*, unsigned long, int, bool, bool) (filters.cpp:427)
==17861==    by 0x1712D8: CryptoPP::FilterWithBufferedInput::Put2(unsigned char const*, unsigned long, int, bool) (filters.h:361)
==17861==    by 0x17960B: CryptoPP::BufferedTransformation::ChannelPut2(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, unsigned char const*, unsigned long, int, bool) (cryptlib.cpp:469)
==17861==    by 0x17A6FD: ChannelMessageEnd (cryptlib.h:2258)
==17861==    by 0x17A6FD: CryptoPP::BufferedTransformation::TransferMessagesTo2(CryptoPP::BufferedTransformation&, unsigned int&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, bool) (cryptlib.cpp:661)
==17861==    by 0x17B183: CryptoPP::BufferedTransformation::TransferAllTo2(CryptoPP::BufferedTransformation&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, bool) (cryptlib.cpp:702)
==17861==    by 0x1770DB: CryptoPP::SourceTemplate<CryptoPP::StringStore>::PumpAll2(bool) (filters.h:1444)

Hence the variable only "leaks" when the program ends, but at that point the OS frees all the memory, so there is no memory leak.