From 0955539cf9bb0755d4190145893a3243c7a2ab97 Mon Sep 17 00:00:00 2001 From: dearblue Date: Fri, 13 Sep 2024 21:44:56 +0900 Subject: [PATCH] Fix use-after-free in `mrb_ary_delete()` `mrb_equal()` may call `obj.==` method internally. Therefore, using an unupdated pointer and length after `mrb_equal()` could result in a read/write to an invalid address. Fresh properties must always be obtained regardless of the result of `mrb_equal()`. Also, `ary_modify()` must be called each time before writing. ref. #6339 --- src/array.c | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/src/array.c b/src/array.c index fd1341403..8899e62e5 100644 --- a/src/array.c +++ b/src/array.c @@ -1551,17 +1551,12 @@ mrb_ary_delete(mrb_state *mrb, mrb_value self) mrb_get_args(mrb, "o&", &obj, &blk); struct RArray *ary = RARRAY(self); - mrb_value *val_ptr = ARY_PTR(ary); - size_t len = ARY_LEN(ary); - mrb_bool modified = FALSE; - mrb_value ret = obj; - int ai = mrb_gc_arena_save(mrb); size_t i = 0; size_t j = 0; - for (; i < len; i++) { - mrb_value elem = val_ptr[i]; + for (; i < ARY_LEN(ary); i++) { + mrb_value elem = ARY_PTR(ary)[i]; if (mrb_equal(mrb, elem, obj)) { mrb_gc_arena_restore(mrb, ai); @@ -1571,12 +1566,13 @@ mrb_ary_delete(mrb_state *mrb, mrb_value self) } if (i != j) { - if (!modified) { - ary_modify(mrb, ary); - val_ptr = ARY_PTR(ary); - modified = TRUE; + if (j >= ARY_LEN(ary)) { + // Since breaking here will further change the array length, + // there is no choice but to raise an exception or return. + mrb_raise(mrb, E_RUNTIME_ERROR, "array modified during delete"); } - val_ptr[j] = elem; + ary_modify(mrb, ary); + ARY_PTR(ary)[j] = elem; } j++;