Fixes to memory operands for complog (#916)

Co-authored-by: Your Name <you@example.com>

4 files changed, 47 insertions, 18 deletions

diff --git a/frida_mode/include/frida_cmplog.h b/frida_mode/include/frida_cmplog.h
index 28864c0e..b620a472 100644
--- a/frida_mode/include/frida_cmplog.h
+++ b/frida_mode/include/frida_cmplog.h
@@ -8,7 +8,7 @@ void cmplog_init(void);
 /* Functions to be implemented by the different architectures */
 void cmplog_instrument(const cs_insn *instr, GumStalkerIterator *iterator);
 
-gboolean cmplog_is_readable(void *addr, size_t size);
+gboolean cmplog_is_readable(guint64 addr, size_t size);
 
 #endif
 
diff --git a/frida_mode/src/cmplog/cmplog.c b/frida_mode/src/cmplog/cmplog.c
index 3fab1951..7b11c350 100644
--- a/frida_mode/src/cmplog/cmplog.c
+++ b/frida_mode/src/cmplog/cmplog.c
@@ -53,7 +53,7 @@ static gboolean cmplog_contains(GumAddress inner_base, GumAddress inner_limit,
 
 }
 
-gboolean cmplog_is_readable(void *addr, size_t size) {
+gboolean cmplog_is_readable(guint64 addr, size_t size) {
 
   if (cmplog_ranges == NULL) FATAL("CMPLOG not initialized");
 
@@ -65,9 +65,9 @@ gboolean cmplog_is_readable(void *addr, size_t size) {
    * is lower than this. This should avoid some overhead when functions are
    * called where one of the parameters is a size, or a some other small value.
    */
-  if (GPOINTER_TO_SIZE(addr) < DEFAULT_MMAP_MIN_ADDR) { return false; }
+  if (addr < DEFAULT_MMAP_MIN_ADDR) { return false; }
 
-  GumAddress inner_base = GUM_ADDRESS(addr);
+  GumAddress inner_base = addr;
   GumAddress inner_limit = inner_base + size;
 
   for (guint i = 0; i < cmplog_ranges->len; i++) {
diff --git a/frida_mode/src/cmplog/cmplog_x64.c b/frida_mode/src/cmplog/cmplog_x64.c
index 9bf09ad5..4d8f243a 100644
--- a/frida_mode/src/cmplog/cmplog_x64.c
+++ b/frida_mode/src/cmplog/cmplog_x64.c
@@ -134,7 +134,8 @@ static guint64 cmplog_read_reg(GumX64CpuContext *ctx, x86_reg reg) {
 
 }
 
-static guint64 cmplog_read_mem(GumX64CpuContext *ctx, x86_op_mem *mem) {
+static gboolean cmplog_read_mem(GumX64CpuContext *ctx, uint8_t size,
+                                x86_op_mem *mem, guint64 *val) {
 
   guint64 base = 0;
   guint64 index = 0;
@@ -145,26 +146,52 @@ static guint64 cmplog_read_mem(GumX64CpuContext *ctx, x86_op_mem *mem) {
   if (mem->index != X86_REG_INVALID) index = cmplog_read_reg(ctx, mem->index);
 
   address = base + (index * mem->scale) + mem->disp;
-  return address;
+
+  if (!cmplog_is_readable(address, size)) { return FALSE; }
+
+  switch (size) {
+
+    case 1:
+      *val = *((guint8 *)address);
+      return TRUE;
+    case 2:
+      *val = *((guint16 *)address);
+      return TRUE;
+    case 4:
+      *val = *((guint32 *)address);
+      return TRUE;
+    case 8:
+      *val = *((guint64 *)address);
+      return TRUE;
+    default:
+      FATAL("Invalid operand size: %d\n", size);
+
+  }
+
+  return FALSE;
 
 }
 
-static guint64 cmplog_get_operand_value(GumCpuContext *context,
-                                        cmplog_ctx_t * ctx) {
+static gboolean cmplog_get_operand_value(GumCpuContext *context,
+                                         cmplog_ctx_t *ctx, guint64 *val) {
 
   switch (ctx->type) {
 
     case X86_OP_REG:
-      return cmplog_read_reg(context, ctx->reg);
+      *val = cmplog_read_reg(context, ctx->reg);
+      return TRUE;
     case X86_OP_IMM:
-      return ctx->imm;
+      *val = ctx->imm;
+      return TRUE;
     case X86_OP_MEM:
-      return cmplog_read_mem(context, &ctx->mem);
+      return cmplog_read_mem(context, ctx->size, &ctx->mem, val);
     default:
       FATAL("Invalid operand type: %d\n", ctx->type);
 
   }
 
+  return FALSE;
+
 }
 
 static void cmplog_call_callout(GumCpuContext *context, gpointer user_data) {
@@ -177,11 +204,11 @@ static void cmplog_call_callout(GumCpuContext *context, gpointer user_data) {
 
   if (((G_MAXULONG - rdi) < 32) || ((G_MAXULONG - rsi) < 32)) return;
 
+  if (!cmplog_is_readable(rdi, 32) || !cmplog_is_readable(rsi, 32)) return;
+
   void *ptr1 = GSIZE_TO_POINTER(rdi);
   void *ptr2 = GSIZE_TO_POINTER(rsi);
 
-  if (!cmplog_is_readable(ptr1, 32) || !cmplog_is_readable(ptr2, 32)) return;
-
   uintptr_t k = address;
 
   k = (k >> 4) ^ (k << 8);
@@ -271,11 +298,13 @@ static void cmplog_handle_cmp_sub(GumCpuContext *context, guint64 operand1,
 static void cmplog_cmp_sub_callout(GumCpuContext *context, gpointer user_data) {
 
   cmplog_pair_ctx_t *ctx = (cmplog_pair_ctx_t *)user_data;
+  guint64            operand1;
+  guint64            operand2;
 
   if (ctx->operand1.size != ctx->operand2.size) FATAL("Operand size mismatch");
 
-  guint64 operand1 = cmplog_get_operand_value(context, &ctx->operand1);
-  guint64 operand2 = cmplog_get_operand_value(context, &ctx->operand2);
+  if (!cmplog_get_operand_value(context, &ctx->operand1, &operand1)) { return; }
+  if (!cmplog_get_operand_value(context, &ctx->operand2, &operand2)) { return; }
 
   cmplog_handle_cmp_sub(context, operand1, operand2, ctx->operand1.size);
 
diff --git a/frida_mode/test/cmplog/GNUmakefile b/frida_mode/test/cmplog/GNUmakefile
index c203fc5e..37c7450c 100644
--- a/frida_mode/test/cmplog/GNUmakefile
+++ b/frida_mode/test/cmplog/GNUmakefile
@@ -41,26 +41,26 @@ $(TEST_CMPLOG_OBJ): $(TEST_CMPLOG_DIR)compcovtest.cc
 
 qemu: $(TEST_CMPLOG_OBJ) $(CMP_LOG_INPUT)
 	$(ROOT)afl-fuzz \
-		-D \
 		-Q \
 		-i $(TEST_DATA_DIR) \
 		-o $(QEMU_OUT) \
 		-c 0 \
 		-l 3AT \
+		-Z \
 		-- \
 			$(TEST_CMPLOG_OBJ) @@
 
 frida: $(TEST_CMPLOG_OBJ) $(CMP_LOG_INPUT)
 	XAFL_FRIDA_INST_RANGES=$(AFL_FRIDA_INST_RANGES) \
 	$(ROOT)afl-fuzz \
-		-D \
 		-O \
 		-i $(TEST_DATA_DIR) \
 		-o $(FRIDA_OUT) \
 		-c 0 \
 		-l 3AT \
+		-Z \
 		-- \
 			$(TEST_CMPLOG_OBJ) @@
 
 clean:
-	rm -rf $(BUILD_DIR)
\ No newline at end of file
+	rm -rf $(BUILD_DIR)