From f65599e216b0d692b461bbcb9a62d8da4aaf912b Mon Sep 17 00:00:00 2001
From: nathanel23 <46408536+nathanel23@users.noreply.github.com>
Date: Sun, 6 Jan 2019 01:06:23 +0100
Subject: [PATCH] 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.
---
 main.cpp     | 2 ++
 src/util.cpp | 9 +++------
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/main.cpp b/main.cpp
index bada05a..cd5faa5 100644
--- a/main.cpp
+++ b/main.cpp
@@ -26,6 +26,8 @@ template<typename T> void set_vm_value(std::map<std::string, bpo::variable_value
 
 int main(int argc, char *argv[])
 {
+    rhash_library_init();
+
     // Constants for option selection with include/exclude
     /* TODO: Add options to give better control for user
              For example: option to select base game and DLC installers separately,
diff --git a/src/util.cpp b/src/util.cpp
index 48105ff..f40aba7 100644
--- a/src/util.cpp
+++ b/src/util.cpp
@@ -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)
 {
     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);
     if (i < 0)
         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)
 {
     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);
     if (i < 0)
         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;
 
     rhash rhash_context;
-    rhash_library_init();
     rhash_context = rhash_init(RHASH_MD5);
     if(!rhash_context)
     {
@@ -149,7 +146,7 @@ int Util::createXML(std::string filepath, uintmax_t chunk_size, std::string xml_
         fclose(infile);
         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++) {
         uintmax_t range_begin = i*chunk_size;