diff --git a/libyara/arena.c b/libyara/arena.c index fe1644e1..772865db 100644 --- a/libyara/arena.c +++ b/libyara/arena.c @@ -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); diff --git a/tests/test-arena.c b/tests/test-arena.c index a0a5da33..69499d87 100644 --- a/tests/test-arena.c +++ b/tests/test-arena.c @@ -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]);