mirror of
https://github.com/VirusTotal/yara.git
synced 2026-10-12 02:10:00 +00:00
check reloc buffer_id before deref in yr_arena_load_stream (#2202)
noticed the relocation loop in yr_arena_load_stream reads b->data before it checks reloc_ref.buffer_id against num_buffers. the id comes straight from the loaded stream, so a crafted reloc entry with a large buffer_id indexes far past the 16-slot buffers array and reads out of bounds. moving the range check ahead of the dereference rejects it as corrupt first. looks like the check order got flipped in #2178. added a regression test in test-arena.c.
This commit is contained in:
+7
-2
@@ -595,12 +595,17 @@ int yr_arena_load_stream(YR_STREAM* stream, YR_ARENA** arena)
|
||||
|
||||
while (yr_stream_read(&reloc_ref, sizeof(reloc_ref), 1, stream) == 1)
|
||||
{
|
||||
if (reloc_ref.buffer_id >= new_arena->num_buffers)
|
||||
{
|
||||
yr_arena_release(new_arena);
|
||||
return ERROR_CORRUPT_FILE;
|
||||
}
|
||||
|
||||
YR_ARENA_BUFFER* b = &new_arena->buffers[reloc_ref.buffer_id];
|
||||
|
||||
if (b->data == NULL ||
|
||||
if (b->data == NULL ||
|
||||
b->used < sizeof(void*) ||
|
||||
b->used > b->size ||
|
||||
reloc_ref.buffer_id >= new_arena->num_buffers ||
|
||||
reloc_ref.offset > b->used - sizeof(void*))
|
||||
{
|
||||
yr_arena_release(new_arena);
|
||||
|
||||
@@ -174,6 +174,53 @@ static void advanced_tests()
|
||||
yr_finalize();
|
||||
}
|
||||
|
||||
// A relocation entry in a saved arena references a buffer by id. When loading
|
||||
// an untrusted stream that id must be validated against num_buffers before the
|
||||
// buffer is touched, otherwise the loader reads out of bounds.
|
||||
static void corrupt_stream_tests()
|
||||
{
|
||||
yr_initialize();
|
||||
|
||||
uint8_t data[64];
|
||||
size_t n = 0;
|
||||
|
||||
// YR_ARENA_FILE_HEADER: magic, version, num_buffers.
|
||||
memcpy(data + n, "YARA", 4); n += 4;
|
||||
data[n++] = YR_ARENA_FILE_VERSION;
|
||||
data[n++] = 1;
|
||||
|
||||
// One YR_ARENA_FILE_BUFFER: offset (8) and size (4).
|
||||
uint64_t offset = 0; memcpy(data + n, &offset, 8); n += 8;
|
||||
uint32_t size = 8; memcpy(data + n, &size, 4); n += 4;
|
||||
|
||||
// Buffer 0 contents.
|
||||
memset(data + n, 0, 8); n += 8;
|
||||
|
||||
// A relocation entry pointing at a buffer id well beyond num_buffers.
|
||||
uint32_t buffer_id = 0x41414141;
|
||||
uint32_t reloc_offset = 0;
|
||||
memcpy(data + n, &buffer_id, 4); n += 4;
|
||||
memcpy(data + n, &reloc_offset, 4); n += 4;
|
||||
|
||||
FILE* fh = fopen("test-arena-corrupt-stream", "w+b");
|
||||
assert_true_expr(fh != NULL);
|
||||
fwrite(data, 1, n, fh);
|
||||
fflush(fh);
|
||||
fseek(fh, 0, SEEK_SET);
|
||||
|
||||
YR_STREAM stream;
|
||||
stream.user_data = fh;
|
||||
stream.read = (YR_STREAM_READ_FUNC) fread;
|
||||
stream.write = (YR_STREAM_WRITE_FUNC) fwrite;
|
||||
|
||||
YR_ARENA* arena = NULL;
|
||||
assert_true_expr(
|
||||
yr_arena_load_stream(&stream, &arena) == ERROR_CORRUPT_FILE);
|
||||
|
||||
fclose(fh);
|
||||
yr_finalize();
|
||||
}
|
||||
|
||||
int main(int argc, char** argv)
|
||||
{
|
||||
int result = 0;
|
||||
@@ -183,6 +230,7 @@ int main(int argc, char** argv)
|
||||
|
||||
basic_tests();
|
||||
advanced_tests();
|
||||
corrupt_stream_tests();
|
||||
|
||||
YR_DEBUG_FPRINTF(
|
||||
1, stderr, "} = %d // %s() in %s\n", result, __FUNCTION__, argv[0]);
|
||||
|
||||
Reference in New Issue
Block a user