VirusTotal / yara

The pattern matching swiss knife
https://virustotal.github.io/yara/
BSD 3-Clause "New" or "Revised" License
8.19k stars 1.44k forks source link

fix process scanning on macos by properly listing memory regions #2033

Closed vthib closed 8 months ago

vthib commented 8 months ago

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 is 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:

This bug was not detected by any tests 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.