Hash Module Memory Leaks while using WinCrypt APIs on Windows

[dlaktionov1]

When libyara is built with HAVE_WINCRYPT_H = 1, the library uses WinCrypt APIs in order to calculate MD5, SHA1 and SHA256 hashes: CryptCreateHash, CryptHashData, CryptGetHashParam and CryptDestroyHash. As can be seen in crypto.h#L69, these API routines are wrapped using generic C macros, such as yr_md5_init, yr_md5_update and yr_md5_final for MD5 hashes, and similar macros for SHA1 and SHA256 hashes.
The documentation for CryptCreateHash API clearly states that:

When you have finished using the hash object, release the handle by calling the CryptDestroyHash function.

This statement poses an important restriction while using the above hash-calculation macros: whenever the yr_hash_init macro is used, one must always call the yr_hash_final macro in order to release the memory resources used by WinCrypt (or more precisely by the corresponding cryptographic providers). Unfortunately, this restriction is not being followed by the implementation of the hash module, due to a logical hash-caching related bug: to avoid calculating the same hashes for the same regions of the scanned sample, the hash module caches the hash calculation results using a module-specific YR_HASH_TABLE (see add_to_cache and get_from_cache). Whenever the hash.md5, hash.sha1 or hash.sha256 entry points are used over non-string data (data_md5, data_sha1, data_sha256), the module follows the same logical pattern:

• Initializing the hash calculation context using the corresponding yr_hash_init macro.
• Performing a cache lookup using get_from_cache and if a cache hit occurs, immediately returning the available hash value.
• Calculating the requested hash value, using the yr_hash_update macro.
• Finalizing the hash calculation using the yr_hash_final macro.
• Caching the calculated hash using add_to_cache.

The problem with this implementation pattern is now obvious: whenever libyara is given a YARA ruleset containing multiple rules which use the same hash-based conditions over the same data regions of the scanned sample, a cache hit will occur and the library will leak all the WinCrypt-related memory allocations which were made by the corresponding yr_hash_init macro calls. In situations where libyara is linked and used by a long-running critical process (such as a Windows service which implements an antimalware scanning solution), this memory leaking bug will end up consuming the entire virtual memory of the service process until the machine physical memory limit is reached and either the service or the entire machine restarts due to lack of memory.
The solution to the problem is obvious as well: once yr_hash_init macro is used, the yr_hash_final macro must be called on every execution branch which returns to the caller (the cache hit scenario and any other error-handling code blocks).
The issue affects all libyara releases since 4.0.x.

[plusvic]

Fixed in #1705