Fix for memory corruption/crash related to rhash

This patch fixes issues with some calls to the rhash C library that
cause subtle memory corruptions. There are two main issues that this
patch addresses:

1) The library init function is called multiple times in various places
within in the Util class. This is not necessary and may cause issues
if the initialization is repeated simultaneously in multiple threads.
Therefore the call was moved to the beginning of the main() function.

2) There is not enough space in the output buffers to store the
terminating NUL character of the hex-formatted hashes. The print
function from the rhash library writes a regular C string to
the output buffer and expects enough space to include the end
marker.

Depending on the memory layout generated by the compiler, this results
in one byte of the stack to be overwritten, which might lead to random
issues that are hard to find. On AArch64 (ARM) with GCC 6.3 the call to
the rhash_print() function happens to destroy the lower byte of the
frame pointer and causes a crash due to an invalid free() operation some
time after the Util::createXML function returned.
This commit is contained in:
nathanel23 2019-01-06 01:06:23 +01:00
parent 9bd416e3b0
commit f65599e216
2 changed files with 5 additions and 6 deletions

View File

@ -26,6 +26,8 @@ template<typename T> void set_vm_value(std::map<std::string, bpo::variable_value
int main(int argc, char *argv[]) int main(int argc, char *argv[])
{ {
rhash_library_init();
// Constants for option selection with include/exclude // Constants for option selection with include/exclude
/* TODO: Add options to give better control for user /* TODO: Add options to give better control for user
For example: option to select base game and DLC installers separately, For example: option to select base game and DLC installers separately,

View File

@ -59,9 +59,8 @@ std::string Util::makeRelativeFilepath(const std::string& path, const std::strin
std::string Util::getFileHash(const std::string& filename, unsigned hash_id) std::string Util::getFileHash(const std::string& filename, unsigned hash_id)
{ {
unsigned char digest[rhash_get_digest_size(hash_id)]; unsigned char digest[rhash_get_digest_size(hash_id)];
char result[rhash_get_hash_length(hash_id)]; char result[rhash_get_hash_length(hash_id) + 1];
rhash_library_init();
int i = rhash_file(hash_id, filename.c_str(), digest); int i = rhash_file(hash_id, filename.c_str(), digest);
if (i < 0) if (i < 0)
std::cerr << "LibRHash error: " << strerror(errno) << std::endl; std::cerr << "LibRHash error: " << strerror(errno) << std::endl;
@ -74,9 +73,8 @@ std::string Util::getFileHash(const std::string& filename, unsigned hash_id)
std::string Util::getChunkHash(unsigned char *chunk, uintmax_t chunk_size, unsigned hash_id) std::string Util::getChunkHash(unsigned char *chunk, uintmax_t chunk_size, unsigned hash_id)
{ {
unsigned char digest[rhash_get_digest_size(hash_id)]; unsigned char digest[rhash_get_digest_size(hash_id)];
char result[rhash_get_hash_length(hash_id)]; char result[rhash_get_hash_length(hash_id) + 1];
rhash_library_init();
int i = rhash_msg(hash_id, chunk, chunk_size, digest); int i = rhash_msg(hash_id, chunk, chunk_size, digest);
if (i < 0) if (i < 0)
std::cerr << "LibRHash error: " << strerror(errno) << std::endl; std::cerr << "LibRHash error: " << strerror(errno) << std::endl;
@ -141,7 +139,6 @@ int Util::createXML(std::string filepath, uintmax_t chunk_size, std::string xml_
std::cout << "Getting MD5 for chunks" << std::endl; std::cout << "Getting MD5 for chunks" << std::endl;
rhash rhash_context; rhash rhash_context;
rhash_library_init();
rhash_context = rhash_init(RHASH_MD5); rhash_context = rhash_init(RHASH_MD5);
if(!rhash_context) if(!rhash_context)
{ {
@ -149,7 +146,7 @@ int Util::createXML(std::string filepath, uintmax_t chunk_size, std::string xml_
fclose(infile); fclose(infile);
return res; return res;
} }
char rhash_result[rhash_get_hash_length(RHASH_MD5)]; char rhash_result[rhash_get_hash_length(RHASH_MD5) + 1];
for (i = 0; i < chunks; i++) { for (i = 0; i < chunks; i++) {
uintmax_t range_begin = i*chunk_size; uintmax_t range_begin = i*chunk_size;