From 8695c845d051636714ed3ca436ba72b83ebd8a6c Mon Sep 17 00:00:00 2001
From: Vincent Thiberville <vthib@pm.me>
Date: Tue, 30 Jan 2024 10:05:09 +0100
Subject: [PATCH] fix process scanning on macos by properly listing memory
 regions (#2033)

The process scanning on macos was broken since 4.2.0 in this commit:

605b2edf07ed8eb9a2c61ba22eb2e7c362f47ba7

Logic was added to properly iterate on memory chunks if a memory region
was too big, but this completely broke the iteration. To retrieve a
memory region, the code basically does:

```
current_begin = 0;
for (;;) {
    vm_address_t addr = current_begin;

    mach_vm_region(..., &addr, &size, ...);

    size_t chunk_size = size - (current_begin - addr);
    ...
}
```

It assumes that `addr` is smaller than `current_begin`, which is never
the case outside of chunking. This leads to an iteration on completely
wrong regions, leading to read that either fail because the starting
addr or length is wrong, or that succeed, but do not cover the whole
region.

For example, with a process memory that looks like:

```
1065a6000-1065f2000
1065f2000-1065f6000
1065f6000-1065fa000
...
600000000000-600008000000
600008000000-600010000000
...
7fe537f00000-7fe538000000
7fe538000000-7fe538800000
```

The very first call would have `current_begin=0` and `addr=0x1065a6000`,
leading to underflow when computing `chunk_size`, and trying to iterate
on the region `0x00000000-0x40000000` (as the max memory chunk is 1GB
per default). this will fail to fetch, and will then iterate on
`0x40000000-0x80000000`, then `0x80000000-0xC0000000`, then
`0xC0000000-0x100000000`. We will finally reach the very first real
memory region, but fail to properly compute the region size with another
underflow, etc.

This commit fixes the issue by distinguishing the two cases:
- either `addr > current_begin` : `current_begin` is not in any memory
  region, and we must fast forward.
- or `addr <= current_begin`: we can compute the current chunk by
  diffing the two values.

This was not tested because the test_process_scan was not running on
macos: trying to scan the sh process always fails even when root, and
the test was thus skipped. I hacked the test locally to check if it
worked by replacing the sh process by something else that we can attach
to, and it showed the scanning fails. With this commit, the test does
pass.

I did not commit any modification to the test as I used a custom binary
for this and it needs to be run as root, but it would probably be a good
idea to adapt it.
---
 libyara/proc/mach.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/libyara/proc/mach.c b/libyara/proc/mach.c
index 8ed3f56658..b3456676bb 100644
--- a/libyara/proc/mach.c
+++ b/libyara/proc/mach.c
@@ -147,7 +147,17 @@ YR_API YR_MEMORY_BLOCK* yr_process_get_next_memory_block(
 
     if (kr == KERN_SUCCESS)
     {
-      size_t chunk_size = size - (size_t) (current_begin - address);
+      size_t chunk_size;
+
+      if (current_begin < address) {
+        // current_begin is outside of any region, and the next region was
+        // returned, so advance to it.
+        current_begin = address;
+        chunk_size = size;
+      } else {
+        // address <= current_begin, compute the size for the current chunk.
+        chunk_size = size - (size_t) (current_begin - address);
+      }
 
       if (((uint64_t) chunk_size) > max_process_memory_chunk)
       {