This fix assumes that the SHA256 of an empty buffer is all zeros. That is not the case. Instead, if there’s no context object when we want to call EVP_DigestFinal_ex(), we should stuff all zeros in the result buffer instead. shows what goes wrong with this fix. It also contains the proper fix.

From the AES code, the diagnosis appears plausible and the corresponding fix looks correct. After this patch, we should have initialized the digest prior to finalizing.

I agree with Greg’s review comments about the stylistic change. Further, I’d like to see a regression test: this seems like a reasonably common thing to do and appears to crash in a default setup.

CODE REVIEW I’m not a security expert, but the patch looks good to me. I reproduce the problem before the patch, and with the patch, no crash. Also, with the patch, no new test failures.

Perhaps in V9_0-branch, we could change:

bool init_EVP_context(std::unique_ptr<EVP_MD_CTX, decltype(&EVP_MD_CTX_destroy)> &ctx);
to

std::unique_ptr<EVP_MD_CTX, decltype(..)> init_EVP_context();

as mutating a std::unique_ptr is a bit of an antipattern, and with RVO, returning one “by value” doesn’t actually copy anything, and just looks cleaner, as the caller can do

my_context = init_EVP_context();

And check my_context for nullness (with the overloaded bool conversion) if needed, e.g.

if (my_context) { …