Skip to content

Commit

Permalink
fix process scanning on macos by properly listing memory regions (#2033)
Browse files Browse the repository at this point in the history
The process scanning on macos was broken since 4.2.0 in this commit:

605b2ed

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.
  • Loading branch information
vthib authored Jan 30, 2024
1 parent 77b5dfa commit 8695c84
Showing 1 changed file with 11 additions and 1 deletion.
12 changes: 11 additions & 1 deletion libyara/proc/mach.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down

0 comments on commit 8695c84

Please sign in to comment.