From be9431a9ef59f1d9165d4b224e9a82ae40e36fd1 Mon Sep 17 00:00:00 2001 From: dearblue Date: Tue, 22 Oct 2024 22:57:49 +0900 Subject: [PATCH] Fix use-after-free by `mrb_gc_unregistor()` Calling `mrb_gc_unregistor()` from `mrb_data_type::dfree` caused a use-after-free deep inside `mrb_close()`. The impetus to investigate was . Currently, when `mrb_close()` is called, all objects are destroyed first. The process is done heap page by heap page, and when all objects belonging to a heap page are destroyed, the heap page is released. If the next heap page contains `RData` objects, the `mrb_gc_unregistor()` function may be called from the `mrb_data_type::dfree` function. At this time, the `mrb_gc_unregistor()` function gets an array object from a Ruby global variable. If the array object belongs to a freed heap page, use-after-free is established by referencing this array object. About the fixes. First of all, there is the fact that the `mrb_gv_get()` function returns `nil` if `mrb->globals` is `NULL`. Therefore, before destroying all objects, free `mrb->globals` and set `mrb->globals` to `NULL` at the same time. Now the `mrb_gv_get()` function will return `nil` to the calling `mrb_gc_unregistor()` function and `mrb_gc_unregistor()` will do nothing more. ref. https://github.com/mruby/mruby/issues/4618 --- src/state.c | 2 +- src/variable.c | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/state.c b/src/state.c index 35ea71e2d..4ead6d9fa 100644 --- a/src/state.c +++ b/src/state.c @@ -185,9 +185,9 @@ mrb_close(mrb_state *mrb) mrb_protect_atexit(mrb); /* free */ + mrb_gc_free_gv(mrb); mrb_gc_destroy(mrb, &mrb->gc); mrb_free_context(mrb, mrb->root_c); - mrb_gc_free_gv(mrb); mrb_free_symtbl(mrb); mrb_free(mrb, mrb); } diff --git a/src/variable.c b/src/variable.c index e6921a7bf..57f2b15aa 100644 --- a/src/variable.c +++ b/src/variable.c @@ -251,8 +251,10 @@ mrb_gc_mark_gv(mrb_state *mrb) void mrb_gc_free_gv(mrb_state *mrb) { - if (mrb->globals) + if (mrb->globals) { iv_free(mrb, mrb->globals); + mrb->globals = NULL; + } } size_t