Skip to content

Commit a190c6a

Browse files
committed
Fix potential memory leak in SignatureVerifier_CryptoAPI
We did not call CryptDestroyHash() on HCRYPTHASH objects since the documentation seemed to indicate this was done for security reasons, which is not important in this case: "To help ensure security, we recommend that hash objects be destroyed after they have been used." However, the Remarks section indicates that they should be destroyed, and a leak showed up in Dr. Memory.
1 parent 8632191 commit a190c6a

File tree

1 file changed

+10
-4
lines changed

1 file changed

+10
-4
lines changed

src/SignatureVerifier_CryptoAPI.cpp

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,30 +40,36 @@ verify(basic_Error & e, HCRYPTPROV hProv, HCRYPTKEY hPubKey, std::string const&
4040
// CryptoAPI assumes things are LSB or whatever, other way around from other people.
4141
for (size_t i = 0, j = sig.size() - 1; i < j; ++i, --j) { std::swap(sig[i], sig[j]); }
4242

43-
HCRYPTHASH hHash;
43+
HCRYPTHASH hHash = 0; // Initialized following https://docs.microsoft.com/sv-se/windows/win32/seccrypto/example-c-program-signing-a-hash-and-verifying-the-hash-signature
4444
if (!CryptCreateHash(hProv, CALG_SHA_256, 0, 0, &hHash)) {
4545
DWORD code = GetLastError();
4646
e.set(api, Subsystem::SignatureVerifier, CRYPT_CREATE_HASH_FAILED, code);
47-
return;
47+
goto cleanup;
4848
}
4949

5050
if (!CryptHashData(hHash, (const BYTE*)message.c_str(), (DWORD)message.size(), 0)) {
5151
DWORD code = GetLastError();
5252
e.set(api, Subsystem::SignatureVerifier, CRYPT_HASH_DATA_FAILED, code);
53-
return;
53+
goto cleanup;
5454
}
5555

5656
if (!CryptVerifySignature(hHash, (const BYTE*)sig.c_str(), (DWORD)sig.size(), hPubKey, NULL, 0)) {
5757
DWORD code = GetLastError();
5858
e.set(api, Subsystem::SignatureVerifier, CRYPT_VERIFY_SIGNATURE_FAILED, code);
59-
return;
59+
goto cleanup;
60+
}
61+
62+
cleanup:
63+
if (hHash) {
64+
CryptDestroyHash(hHash);
6065
}
6166
}
6267

6368
SignatureVerifier_CryptoAPI::SignatureVerifier_CryptoAPI(basic_Error & e) : hProv_{}, hPubKey_{} { }
6469

6570
void SignatureVerifier_CryptoAPI::init(basic_Error & e)
6671
{
72+
// TODO: Move this to the constructor now that we pass in the error object
6773
if (!CryptAcquireContext(&this->hProv_, NULL, MS_ENH_RSA_AES_PROV, PROV_RSA_AES, CRYPT_VERIFYCONTEXT)) {
6874
DWORD code = GetLastError();
6975
e.set(api::main(), errors::Subsystem::SignatureVerifier, CRYPT_ACQUIRE_CONTEXT_FAILED, code);

0 commit comments

Comments
 (0)