diff --git a/libyara/exec.c b/libyara/exec.c index 275230792d..349c4a27e5 100644 --- a/libyara/exec.c +++ b/libyara/exec.c @@ -33,6 +33,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include #include #include +#include #include #include #include @@ -98,6 +99,18 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. } \ } +// Make sure that the instruction pointer stays within the code section. A +// crafted compiled-rules file can carry a jump whose offset moves ip outside +// the code buffer, and the dispatch loop would then read the opcode out of +// bounds. +#define ensure_within_code(x) \ + if ((x) < code_start || (x) >= code_end) \ + { \ + stop = true; \ + result = ERROR_INTERNAL_FATAL_ERROR; \ + break; \ + } + #define check_object_canary(o) \ if (o->canary != context->canary) \ { \ @@ -419,7 +432,16 @@ int yr_execute_code(YR_SCAN_CONTEXT* context) { YR_DEBUG_FPRINTF(2, stderr, "+ %s() {\n", __FUNCTION__); - const uint8_t* ip = context->rules->code_start; + const uint8_t* code_start = context->rules->code_start; + const uint8_t* ip = code_start; + +#if YR_PARANOID_EXEC + // One byte past the last instruction of the code section, used by + // ensure_within_code to reject a jump that leaves the code buffer. + const uint8_t* code_end = code_start + + yr_arena_get_current_offset( + context->rules->arena, YR_CODE_SECTION); +#endif YR_VALUE mem[MEM_SIZE]; YR_VALUE args[YR_MAX_FUNCTION_ARGS]; @@ -485,6 +507,10 @@ int yr_execute_code(YR_SCAN_CONTEXT* context) while (!stop) { +#if YR_PARANOID_EXEC + ensure_within_code(ip); +#endif + // Read the opcode from the address indicated by the instruction pointer. opcode = *ip; diff --git a/tests/test-exec.c b/tests/test-exec.c index 0dd4d70aa2..c1fd04849b 100644 --- a/tests/test-exec.c +++ b/tests/test-exec.c @@ -164,6 +164,79 @@ static void test_push_rule_index_overflow(void) free(buffer.data); } +// The conditional jump opcodes carry a 4-byte offset taken straight from the +// compiled-rules bytecode. jmp_if added that offset to the instruction pointer +// without checking that the result stays inside the code section, and the +// dispatch loop then read the next opcode from the resulting address. A +// compiled rule hand-crafted so that a taken jump points far outside the code +// buffer makes the scan read out of bounds. The jump target must be rejected. +static void test_jump_offset_overflow(void) +{ + YR_COMPILER* compiler = NULL; + YR_RULES* rules = NULL; + YR_RULES* loaded = NULL; + + if (yr_compiler_create(&compiler) != ERROR_SUCCESS) + exit(EXIT_FAILURE); + + // "false and true" is emitted as OP_PUSH_8 0 (0x3F 0x00) followed by an + // OP_JFALSE (0x2F) whose 4-byte offset skips over the second operand. The + // pushed value is false, so the jump is always taken. + if (yr_compiler_add_string( + compiler, "rule x { condition: false and true }", NULL) != 0) + exit(EXIT_FAILURE); + + if (yr_compiler_get_rules(compiler, &rules) != ERROR_SUCCESS) + exit(EXIT_FAILURE); + + MEM_STREAM buffer = {0}; + YR_STREAM out = {.user_data = &buffer, .read = mem_read, .write = mem_write}; + + if (yr_rules_save_stream(rules, &out) != ERROR_SUCCESS) + exit(EXIT_FAILURE); + + // Find the OP_PUSH_8 0 / OP_JFALSE pair and bump the jump offset to a value + // that lands well past the end of the code buffer. + int tampered = 0; + + for (size_t i = 0; i + 6 < buffer.size; i++) + { + if (buffer.data[i] == 0x3F && buffer.data[i + 1] == 0x00 && + buffer.data[i + 2] == 0x2F) + { + buffer.data[i + 3] = 0x00; + buffer.data[i + 4] = 0x00; + buffer.data[i + 5] = 0x01; + buffer.data[i + 6] = 0x00; + tampered = 1; + break; + } + } + + assert(tampered); + + yr_rules_destroy(rules); + yr_compiler_destroy(compiler); + + buffer.pos = 0; + YR_STREAM in = {.user_data = &buffer, .read = mem_read, .write = mem_write}; + + if (yr_rules_load_stream(&in, &loaded) != ERROR_SUCCESS) + exit(EXIT_FAILURE); + + uint8_t data[16] = {0}; + + // Without the bounds check this scan follows the jump out of the code buffer + // and reads the next opcode out of bounds. The jump must be detected instead. + int result = yr_rules_scan_mem( + loaded, data, sizeof(data), 0, scan_callback, NULL, 0); + + assert(result == ERROR_INTERNAL_FATAL_ERROR); + + yr_rules_destroy(loaded); + free(buffer.data); +} + int main(int argc, char** argv) { YR_DEBUG_INITIALIZE(); @@ -172,6 +245,7 @@ int main(int argc, char** argv) yr_initialize(); test_push_rule_index_overflow(); + test_jump_offset_overflow(); yr_finalize();