Check for integer overflows when allocating memory

Context: android/ndk#294
Context: android/ndk#295
Context: https://bugs.llvm.org/show_bug.cgi?id=16404

Add a couple of functions to `Utils` taking advantage of the compiler builtin
functions to perform integer addition and multiplication while checking for
overflows. If an overflow is detected, the application is terminated.

Due to a bug in Android NDK's clang, the multiplication is not performed using
"open" types but rather, currently, only `size_t` for the multiplication
operands.

Using a template for them would result in a link error as the compiler would
generate code to use 128-bit integers to perform the operation, attempting to
call the `__muloti4` intrinsic function which is usually defined in `libgcc`,
`libcompiler_rt` or `libcompiler_rt-extras` libraries. In the NDK case the
64-bit targets do not contain implementation of the function in neither of the
libraries mentioned above.

src/monodroid/jni/android-system.cc

@@ -148,8 +148,9 @@ AndroidSystem::add_system_property (const char *name, const char *value)
 		return;
 	}
 
-	size_t name_len  = strlen (name);
-	p = reinterpret_cast<BundledProperty*> (malloc (sizeof ( BundledProperty) + name_len + 1));
+	size_t name_len  = strlen (name) + 1;
+	size_t alloc_size = utils.add_with_overflow_check<size_t> (sizeof (BundledProperty), name_len);
+	p = reinterpret_cast<BundledProperty*> (malloc (alloc_size));
 	if (p == nullptr)
 		return;
 
@@ -227,8 +228,9 @@ AndroidSystem::_monodroid__system_property_get (const char *name, char *sp_value
 
 	char *buf = nullptr;
 	if (sp_value_len < PROP_VALUE_MAX + 1) {
+		size_t alloc_size = utils.add_with_overflow_check<size_t> (PROP_VALUE_MAX, 1);
 		log_warn (LOG_DEFAULT, "Buffer to store system property may be too small, will copy only %u bytes", sp_value_len);
-		buf = new char [PROP_VALUE_MAX + 1];
+		buf = new char [alloc_size];
 	}
 
 	int len = __system_property_get (name, buf ? buf : sp_value);
@@ -262,7 +264,8 @@ AndroidSystem::monodroid_get_system_property (const char *name, char **value)
 	}
 
 	if (len >= 0 && value) {
-		*value = new char [static_cast<size_t>(len) + 1];
+		size_t alloc_size = utils.add_with_overflow_check<size_t> (static_cast<size_t>(len), 1);
+		*value = new char [alloc_size];
 		if (*value == nullptr)
 			return -len;
 		if (len > 0)
@@ -283,7 +286,7 @@ AndroidSystem::monodroid_read_file_into_memory (const char *path, char **value)
 	if (fp != nullptr) {
 		struct stat fileStat;
 		if (fstat (fileno (fp), &fileStat) == 0) {
-			r = static_cast<size_t>(fileStat.st_size) + 1;
+			r = utils.add_with_overflow_check<size_t> (static_cast<size_t>(fileStat.st_size), 1);
 			if (value && (*value = new char[r])) {
 				fread (*value, 1, static_cast<size_t>(fileStat.st_size), fp);
 			}
@@ -316,7 +319,8 @@ AndroidSystem::_monodroid_get_system_property_from_file (const char *path, char
 		return file_size + 1;
 	}
 
-	*value = new char[file_size + 1];
+	size_t alloc_size = utils.add_with_overflow_check<size_t> (file_size, 1);
+	*value = new char[alloc_size];
 	if (!(*value)) {
 		fclose (fp);
 		return file_size + 1;

src/monodroid/jni/embedded-assemblies.cc

@@ -70,7 +70,9 @@ EmbeddedAssemblies::open_from_bundles (MonoAssemblyName* aname, bool ref_only)
 	size_t name_len = culture == nullptr ? 0 : strlen (culture) + 1;
 	name_len += sizeof (".exe");
 	name_len += strlen (asmname);
-	char *name = new char [name_len + 1];
+
+	size_t alloc_size = utils.add_with_overflow_check<size_t> (name_len, 1);
+	char *name = new char [alloc_size];
 	name [0] = '\0';
 
 	if (culture != nullptr && *culture != '\0') {
@@ -457,7 +459,8 @@ EmbeddedAssemblies::gather_bundled_assemblies_from_apk (const char* apk, monodro
 			if (entry_is_overridden)
 				continue;
 
-			bundled_assemblies = reinterpret_cast<MonoBundledAssembly**> (utils.xrealloc (bundled_assemblies, sizeof(void*) * (bundled_assemblies_count + 1)));
+			size_t alloc_size = utils.multiply_with_overflow_check<size_t> (sizeof(void*), bundled_assemblies_count + 1);
+			bundled_assemblies = reinterpret_cast<MonoBundledAssembly**> (utils.xrealloc (bundled_assemblies, alloc_size));
 			cur = bundled_assemblies [bundled_assemblies_count] = reinterpret_cast<MonoBundledAssembly*> (utils.xcalloc (1, sizeof (MonoBundledAssembly)));
 			++bundled_assemblies_count;
 
@@ -550,7 +553,8 @@ EmbeddedAssemblies::register_from (const char *apk_file, monodroid_should_regist
 	log_info (LOG_ASSEMBLY, "Package '%s' contains %i assemblies", apk_file, bundled_assemblies_count - prev);
 
 	if (bundled_assemblies) {
-		bundled_assemblies  = reinterpret_cast <MonoBundledAssembly**> (utils.xrealloc (bundled_assemblies, sizeof(void*)*(bundled_assemblies_count + 1)));
+		size_t alloc_size = utils.multiply_with_overflow_check<size_t> (sizeof(void*), bundled_assemblies_count + 1);
+		bundled_assemblies  = reinterpret_cast <MonoBundledAssembly**> (utils.xrealloc (bundled_assemblies, alloc_size));
 		bundled_assemblies [bundled_assemblies_count] = nullptr;
 	}
 

src/monodroid/jni/monodroid-glue.cc

@@ -670,8 +670,9 @@ parse_runtime_args (char *runtime_args, RuntimeOptions *options)
 				sep = strchr (arg, ':');
 				if (sep != nullptr) {
 					size_t arg_len = static_cast<size_t>(sep - arg);
-					host = new char [arg_len + 1];
-					memset (host, 0x00, arg_len + 1);
+					size_t alloc_size = utils.add_with_overflow_check<size_t> (arg_len, 1);
+					host = new char [alloc_size];
+					memset (host, 0x00, alloc_size);
 					strncpy (host, arg, arg_len);
 					arg = sep+1;
 
@@ -1503,7 +1504,8 @@ monodroid_profiler_load (const char *libmono_path, const char *desc, const char
 
 	if (col != nullptr) {
 		size_t name_len = static_cast<size_t>(col - desc);
-		mname = new char [name_len + 1];
+		size_t alloc_size = utils.add_with_overflow_check<size_t> (name_len, 1);
+		mname = new char [alloc_size];
 		strncpy (mname, desc, name_len);
 		mname [name_len] = 0;
 	} else {
@@ -1573,7 +1575,8 @@ set_profile_options (JNIEnv *env)
 			extension = utils.strdup_new ("aotprofile");
 		else {
 			size_t len = col != nullptr ? static_cast<size_t>(col - value) : strlen (value);
-			extension = new char [len + 1];
+			size_t alloc_size = utils.add_with_overflow_check<size_t> (len, 1);
+			extension = new char [alloc_size];
 			strncpy (extension, value, len);
 			extension [len] = '\0';
 		}

src/monodroid/jni/monodroid-networkinfo.cc

@@ -32,6 +32,7 @@
 #include "monodroid.h"
 #include "monodroid-glue.h"
 #include "util.h"
+#include "globals.h"
 
 using namespace xamarin::android;
 
@@ -167,7 +168,8 @@ _monodroid_get_dns_servers (void **dns_servers_array)
 	if (count <= 0)
 		return 0;
 
-	char **ret = (char**)malloc (sizeof (char*) * static_cast<size_t>(count));
+	size_t alloc_size = utils.multiply_with_overflow_check<size_t> (sizeof (char*), static_cast<size_t>(count));
+	char **ret = (char**)malloc (alloc_size);
 	char **p = ret;
 	for (int i = 0; i < 8; i++) {
 		if (!dns_servers [i])

src/monodroid/jni/util.cc

@@ -103,7 +103,7 @@ Util::path_combine (const char *path1, const char *path2)
 	if (path2 == nullptr)
 		return strdup_new (path1);
 
-	size_t len = strlen (path1) + strlen (path2) + 2;
+	size_t len = add_with_overflow_check<size_t> (strlen (path1), strlen (path2) + 2);
 	char *ret = new char [len];
 	*ret = '\0';
 
@@ -117,9 +117,13 @@ Util::path_combine (const char *path1, const char *path2)
 void
 Util::add_to_vector (char ***vector, size_t size, char *token)
 {
-	*vector = *vector == nullptr ?
-		(char **)xmalloc(2 * sizeof(*vector)) :
-		(char **)xrealloc(*vector, (size + 1) * sizeof(*vector));
+	if (*vector == nullptr) {
+		size = 2;
+		*vector = (char **)xmalloc(size * sizeof(*vector));
+	} else {
+		size_t alloc_size = utils.multiply_with_overflow_check<size_t> (sizeof(*vector), size + 1);
+		*vector = (char **)xrealloc(*vector, alloc_size);
+	}
 
 	(*vector)[size - 1] = token;
 }
@@ -165,7 +169,8 @@ Util::monodroid_strsplit (const char *str, const char *delimiter, size_t max_tok
 
 			if (*str) {
 				size_t toklen = static_cast<size_t>((str - c));
-				token = new char [toklen + 1];
+				size_t alloc_size = add_with_overflow_check<size_t> (toklen, 1);
+				token = new char [alloc_size];
 				strncpy (token, c, toklen);
 				token [toklen] = '\0';
 

src/monodroid/jni/util.h

@@ -227,6 +227,44 @@ namespace xamarin { namespace android
 			return (log_categories & category) != 0;
 		}
 
+		template<typename Ret, typename P1, typename P2>
+		inline Ret add_with_overflow_check (P1 a, P2 b) const
+		{
+			Ret ret;
+
+			if (XA_UNLIKELY (__builtin_add_overflow (a, b, &ret))) {
+				log_fatal (LOG_DEFAULT, "Integer overflow on addition");
+				exit (FATAL_EXIT_OUT_OF_MEMORY);
+				return static_cast<Ret>(0);
+			}
+
+			return ret;
+		}
+
+		// Can't use templates as above with add_with_oveflow because of a bug in the clang compiler
+		// shipped with the NDK:
+		//
+		//   https://github.com/android-ndk/ndk/issues/294
+		//   https://github.com/android-ndk/ndk/issues/295
+		//   https://bugs.llvm.org/show_bug.cgi?id=16404
+		//
+		// Using templated parameter types for `a` and `b` would make clang generate that tries to
+		// use 128-bit integers and thus output code that calls `__muloti4` and so linking would
+		// fail
+		//
+		template<typename Ret>
+		inline Ret multiply_with_overflow_check (size_t a, size_t b) const
+		{
+			Ret ret;
+
+			if (XA_UNLIKELY (__builtin_mul_overflow (a, b, &ret))) {
+				log_fatal (LOG_DEFAULT, "Integer overflow on multiplication");
+				exit (FATAL_EXIT_OUT_OF_MEMORY);
+				return static_cast<Ret>(0);
+			}
+
+			return ret;
+		}
 	private:
 		//char *monodroid_strdup_printf (const char *format, va_list vargs);
 		void  add_to_vector (char ***vector, size_t size, char *token);

src/monodroid/jni/xamarin_getifaddrs.cc

@@ -532,7 +532,8 @@ parse_netlink_reply (netlink_session *session, struct _monodroid_ifaddrs **ifadd
 	size_t buf_size = static_cast<size_t>(getpagesize ());
 	log_debug (LOG_NETLINK, "receive buffer size == %d", buf_size);
 
-	response = (unsigned char*)malloc (sizeof (*response) * buf_size);
+	size_t alloc_size = utils.multiply_with_overflow_check<size_t> (sizeof(*response), buf_size);
+	response = (unsigned char*)malloc (alloc_size);
 	ssize_t length = 0;
 	if (!response) {
 		goto cleanup;
@@ -869,7 +870,8 @@ get_link_address (const struct nlmsghdr *message, struct _monodroid_ifaddrs **if
 				}
 
 				if (payload_size > 0) {
-					ifa->ifa_name = (char*)malloc (payload_size + room_for_trailing_null);
+					size_t alloc_size = utils.add_with_overflow_check<size_t> (payload_size, room_for_trailing_null);
+					ifa->ifa_name = (char*)malloc (alloc_size);
 					if (!ifa->ifa_name) {
 						goto error;
 					}