fix: few UBs; fix: missing error handler for malloc

This commit fixes some few UBs (Undefined Behaviors) based on numerous sanitizers, and also adds the missing error handling for a "malloc" call.

Author: ThePedroo

loader/src/ptracer/ptracer.cpp

@@ -100,7 +100,12 @@ bool inject_on_main(int pid, const char *lib_path) {
     For arm32 compatibility, we set the last bit to the same as the entry address
   */
 
-  uintptr_t break_addr = (-0x05ec1cff & ~1) | ((uintptr_t)entry_addr & 1);
+  /* INFO: (-0x0F & ~1) is a value below zero, while the one after "|"
+            is an unsigned (must be 0 or greater) value, so we must
+            cast the second value to signed long (intptr_t) to avoid
+            undefined behavior.
+  */
+  uintptr_t break_addr = (uintptr_t)((intptr_t)(-0x0F & ~1) | (intptr_t)((uintptr_t)entry_addr & 1));
   if (!write_proc(pid, (uintptr_t)addr_of_entry_addr, &break_addr, sizeof(break_addr))) return false;
 
   ptrace(PTRACE_CONT, pid, 0, 0);
@@ -110,7 +115,7 @@ bool inject_on_main(int pid, const char *lib_path) {
   if (WIFSTOPPED(status) && WSTOPSIG(status) == SIGSEGV) {
     if (!get_regs(pid, regs)) return false;
 
-    if (static_cast<uintptr_t>(regs.REG_IP & ~1) != (break_addr & ~1)) {
+    if (((int)regs.REG_IP & ~1) != ((int)break_addr & ~1)) {
       LOGE("stopped at unknown addr %p", (void *) regs.REG_IP);
 
       return false;
@@ -184,8 +189,14 @@ bool inject_on_main(int pid, const char *lib_path) {
       }
 
       /* NOTICE: C++ -> C */
-      char *err = (char *)malloc(dlerror_len + 1);
-      read_proc(pid, (uintptr_t) dlerror_str_addr, err, dlerror_len);
+      char *err = (char *)malloc((dlerror_len + 1) * sizeof(char));
+      if (err == NULL) {
+        LOGE("malloc err");
+
+        return false;
+      }
+
+      read_proc(pid, dlerror_str_addr, err, dlerror_len + 1);
 
       LOGE("dlerror info %s", err);
 

loader/src/ptracer/utils.cpp

@@ -313,7 +313,11 @@ void *find_func_addr(std::vector<MapInfo> &local_info, std::vector<MapInfo> &rem
 
 /* WARNING: C++ keyword */
 void align_stack(struct user_regs_struct &regs, long preserve) {
-  regs.REG_SP = (regs.REG_SP - preserve) & ~0xf;
+  /* INFO: ~0xf is a negative value, and REG_SP is unsigned,
+            so we must cast REG_SP to signed type before subtracting
+            then cast back to unsigned type.
+  */
+  regs.REG_SP = (uintptr_t)((intptr_t)(regs.REG_SP - preserve) & ~0xf);
 }
 
 /* WARNING: C++ keyword */

zygiskd/src/companion.c

@@ -54,6 +54,7 @@ void *entry_thread(void *arg) {
   pthread_exit(NULL);
 }
 
+/* WARNING: Dynamic memory based */
 void entry(int fd) {
   LOGI("New companion entry.\n - Client fd: %d\n", fd);
 

zygiskd/src/root_impl/kernelsu.c

@@ -9,7 +9,11 @@
 
 #include "kernelsu.h"
 
-#define KERNEL_SU_OPTION 0xdeadbeef
+/* INFO: It would be presumed it is a unsigned int,
+           so we need to cast it to signed int to
+           avoid any potential UB.
+*/
+#define KERNEL_SU_OPTION (signed int)0xdeadbeef
 
 #define CMD_GET_VERSION 2
 #define CMD_UID_GRANTED_ROOT 12

zygiskd/src/zygiskd.c

@@ -360,6 +360,7 @@ struct __attribute__((__packed__)) MsgHead {
   char data[0];
 };
 
+/* WARNING: Dynamic memory based */
 void zygiskd_start(char *restrict argv[]) {
   LOGI("Welcome to ReZygisk %s Zygiskd!\n", ZKSU_VERSION);