From 46bf55b3375941f1b7d375bd949c8f4561bb137f Mon Sep 17 00:00:00 2001 From: jmestwa-coder Date: Wed, 19 Aug 2026 14:59:48 +0530 Subject: [PATCH] guard repeat-end against empty stack in _yr_re_fiber_sync (#2240) The RE_OPCODE_REPEAT_END handler in _yr_re_fiber_sync does `fiber->stack[fiber->sp]++` without checking that sp is not negative: - sp starts at -1 and only a preceding REPEAT_START advances it to 0 - a hand-crafted .yrc can reach REPEAT_END with no matching REPEAT_START, so `stack[-1]++` writes into the fiber's `next` pointer and the later fiber traversal follows the corrupted value - the REPEAT_START push already carries the paranoid upper-bound guard, only this lower bound was missing Added the sp < 0 guard under YR_PARANOID_EXEC. A regexp of a single REPEAT_END opcode gives an ASAN SEGV in _yr_re_fiber_split before the patch and a clean ERROR_INTERNAL_FATAL_ERROR after; regression test added in test-re-split.c. --- libyara/re.c | 10 ++++++++ tests/test-re-split.c | 53 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/libyara/re.c b/libyara/re.c index 6174af14..d161acf0 100644 --- a/libyara/re.c +++ b/libyara/re.c @@ -1573,6 +1573,16 @@ static int _yr_re_fiber_sync( case RE_OPCODE_REPEAT_END_UNGREEDY: repeat_args = (RE_REPEAT_ARGS*) (fiber->ip + 1); + +#if YR_PARANOID_EXEC + // A REPEAT_END with no matching REPEAT_START leaves sp at -1 and the + // access below would read and write before the stack. In normal + // conditions this never happens, but it can with compiled rules that + // have been hand-crafted by a malicious actor. + if (fiber->sp < 0) + return ERROR_INTERNAL_FATAL_ERROR; +#endif + fiber->stack[fiber->sp]++; if (fiber->stack[fiber->sp] < repeat_args->min) diff --git a/tests/test-re-split.c b/tests/test-re-split.c index 0f49f867..52fe94ac 100644 --- a/tests/test-re-split.c +++ b/tests/test-re-split.c @@ -149,6 +149,58 @@ static void test_repeat_stack_overflow(void) } } +// A compiled rule hand-crafted by a malicious actor can carry a regexp whose +// bytecode reaches a REPEAT_END with no matching REPEAT_START. The compiler +// always emits a REPEAT_START (which pushes onto the fiber's repeat stack) +// before the matching REPEAT_END, so the stack pointer is never negative for +// legitimate rules, but loaded bytecode is not validated. _yr_re_fiber_sync +// increments stack[sp] on REPEAT_END, so such a stream must be rejected +// instead of writing before the stack with sp still -1. +static void test_repeat_stack_underflow(void) +{ + // A single REPEAT_END (opcode byte followed by RE_REPEAT_ARGS, a packed + // { uint16 min; uint16 max; int32 offset } of 8 bytes) with no preceding + // REPEAT_START, followed by a MATCH. + uint8_t code[1 + 8 + 1]; + uint16_t min = 1; + uint16_t max = 2; + int32_t offset = 0; + + code[0] = RE_OPCODE_REPEAT_END_GREEDY; + memcpy(code + 1, &min, sizeof(min)); + memcpy(code + 3, &max, sizeof(max)); + memcpy(code + 5, &offset, sizeof(offset)); + code[9] = RE_OPCODE_MATCH; + + YR_SCAN_CONTEXT context; + memset(&context, 0, sizeof(context)); + + uint8_t input[1] = {0}; + int matches = 0; + + int result = yr_re_exec( + &context, + code, + input, + sizeof(input), + 0, + RE_FLAGS_SCAN, + re_match_callback, + NULL, + &matches); + + assert(result == ERROR_INTERNAL_FATAL_ERROR); + + RE_FIBER* fiber = context.re_fiber_pool.fibers.head; + + while (fiber != NULL) + { + RE_FIBER* next = fiber->next; + yr_free(fiber); + fiber = next; + } +} + int main(int argc, char** argv) { int result = 0; @@ -201,6 +253,7 @@ int main(int argc, char** argv) test_split_id_overflow(); test_repeat_stack_overflow(); + test_repeat_stack_underflow(); yr_finalize();