The iterators on memory blocks is unsafe as it uses pointers on freed objects.
The mem_block_iterator_first and mem_block_iterator_next both have this code:
let mut mem_block = ...;
match mem_block.as_mut() {
Some(mem_block) {
context.mem_block.write(mem_block.as_yara());
context.mem_block.as_mut_ptr()
}
...
}
This is unsafe because the "as_yara" method creates an object that keeps a pointer to the mem_block object, which is freed just afterwards.
This can be seen easily by running the tests in valgrind, where the tests using the mem blocks api have this error:
==23549== Invalid read of size 8
==23549== at 0x1CAAD4: yara::internals::iterator::mem_block_fetch_data (iterator.rs:160)
==23549== by 0x1E8A06: yr_scanner_scan_mem_blocks (scanner.c:479)
==23549== by 0x152458: yara::internals::scan::scanner_scan_mem_blocks_inner (scan.rs:269)
==23549== by 0x1521BF: yara::internals::scan::scanner_scan_mem_blocks (scan.rs:248)
...
The mem_block_fetch_data dereferences the pointer to the freed object, leading to an invalid read.
This is fixed in this commit by avoiding to keep this pointer, and instead directly storing the pointer to the slice data, since this is the value we want to return in mem_block_fetch_data.
I won't pretend that this makes this API safe, since this has to use the yara memory block iterator API, which is very unsafe in how it is declared, and its safety depends on knowledge on how those objects are used internally in YARA. But at least valgrind no longer complains :)
The iterators on memory blocks is unsafe as it uses pointers on freed objects.
The
mem_block_iterator_firstandmem_block_iterator_nextboth have this code:This is unsafe because the "as_yara" method creates an object that keeps a pointer to the mem_block object, which is freed just afterwards.
This can be seen easily by running the tests in valgrind, where the tests using the mem blocks api have this error:
The
mem_block_fetch_datadereferences the pointer to the freed object, leading to an invalid read.This is fixed in this commit by avoiding to keep this pointer, and instead directly storing the pointer to the slice data, since this is the value we want to return in
mem_block_fetch_data.I won't pretend that this makes this API safe, since this has to use the yara memory block iterator API, which is very unsafe in how it is declared, and its safety depends on knowledge on how those objects are used internally in YARA. But at least valgrind no longer complains :)