Skip to content

Commit a7f351a

Browse files
authored
Avoid infinite loops when iterating macho regions (#2145)
There was a possible (and trivial to trigger) infinite loop in yara that can be caused by quiting an application while we're enumerating its memory regions. When this happens, vm_region_64 will start returning an error (that isn't KERN_INVALID_ADDRESS), which yr_process_get_next_memory_block reacted to by infinite looping. Generally speaking, the loop in that function seems completely unnecessary, and as such was removed to make the control flow easier to reason about. This can be reproduced easily by running sudo yara test.yar <pid> on a large application, and quitting the app while the scan is running. This isn't 100% consistent, but more often than not I would end up with the infinite loop.
1 parent dca4940 commit a7f351a

File tree

1 file changed

+37
-38
lines changed

1 file changed

+37
-38
lines changed

libyara/proc/mach.c

+37-38
Original file line numberDiff line numberDiff line change
@@ -132,49 +132,48 @@ YR_API YR_MEMORY_BLOCK* yr_process_get_next_memory_block(
132132

133133
iterator->last_error = ERROR_SUCCESS;
134134

135-
do
135+
info_count = VM_REGION_BASIC_INFO_COUNT_64;
136+
137+
kr = vm_region_64(
138+
proc_info->task,
139+
&address,
140+
&size,
141+
VM_REGION_BASIC_INFO,
142+
(vm_region_info_t) &info,
143+
&info_count,
144+
&object);
145+
146+
if (kr == KERN_INVALID_ADDRESS)
136147
{
137-
info_count = VM_REGION_BASIC_INFO_COUNT_64;
138-
139-
kr = vm_region_64(
140-
proc_info->task,
141-
&address,
142-
&size,
143-
VM_REGION_BASIC_INFO,
144-
(vm_region_info_t) &info,
145-
&info_count,
146-
&object);
147-
148-
if (kr == KERN_SUCCESS)
149-
{
150-
size_t chunk_size;
151-
152-
if (current_begin < address) {
153-
// current_begin is outside of any region, and the next region was
154-
// returned, so advance to it.
155-
current_begin = address;
156-
chunk_size = size;
157-
} else {
158-
// address <= current_begin, compute the size for the current chunk.
159-
chunk_size = size - (size_t) (current_begin - address);
160-
}
161-
162-
if (((uint64_t) chunk_size) > max_process_memory_chunk)
163-
{
164-
chunk_size = (size_t) max_process_memory_chunk;
165-
}
166-
167-
context->current_block.base = (size_t) current_begin;
168-
context->current_block.size = chunk_size;
169-
170-
return &context->current_block;
171-
}
148+
// The end of the address space has been reached.
149+
return NULL;
150+
}
151+
if (kr != KERN_SUCCESS)
152+
{
153+
iterator->last_error = ERROR_COULD_NOT_READ_PROCESS_MEMORY;
154+
return NULL;
155+
}
156+
157+
size_t chunk_size;
172158

159+
if (current_begin < address) {
160+
// current_begin is outside of any region, and the next region was
161+
// returned, so advance to it.
173162
current_begin = address;
163+
chunk_size = size;
164+
} else {
165+
// address <= current_begin, compute the size for the current chunk.
166+
chunk_size = size - (size_t) (current_begin - address);
167+
}
174168

175-
} while (kr != KERN_INVALID_ADDRESS);
169+
if (((uint64_t) chunk_size) > max_process_memory_chunk)
170+
{
171+
chunk_size = (size_t) max_process_memory_chunk;
172+
}
176173

177-
return NULL;
174+
context->current_block.base = (size_t) current_begin;
175+
context->current_block.size = chunk_size;
176+
return &context->current_block;
178177
}
179178

180179
YR_API YR_MEMORY_BLOCK* yr_process_get_first_memory_block(

0 commit comments

Comments
 (0)