diff --git a/ext/sqlite3/aggregator.c b/ext/sqlite3/aggregator.c index 9a04aa64..8f790201 100644 --- a/ext/sqlite3/aggregator.c +++ b/ext/sqlite3/aggregator.c @@ -119,6 +119,7 @@ rb_sqlite3_aggregator_step(sqlite3_context *ctx, int argc, sqlite3_value **argv) VALUE inst = rb_sqlite3_aggregate_instance(ctx); VALUE handler_instance = rb_iv_get(inst, "-handler_instance"); VALUE *params = NULL; + VALUE params_handle = 0; VALUE one_param; int exc_status = NUM2INT(rb_iv_get(inst, "-exc_status")); int i; @@ -132,7 +133,9 @@ rb_sqlite3_aggregator_step(sqlite3_context *ctx, int argc, sqlite3_value **argv) params = &one_param; } if (argc > 1) { - params = xcalloc((size_t)argc, sizeof(VALUE)); + /* ALLOCV memory is conservatively marked, so stored VALUEs survive a + * GC raised by a later sqlite3val2rb call. */ + params = ALLOCV_N(VALUE, params_handle, argc); for (i = 0; i < argc; i++) { params[i] = sqlite3val2rb(argv[i]); } @@ -140,7 +143,7 @@ rb_sqlite3_aggregator_step(sqlite3_context *ctx, int argc, sqlite3_value **argv) rb_sqlite3_protected_funcall( handler_instance, rb_intern("step"), argc, params, &exc_status); if (argc > 1) { - xfree(params); + ALLOCV_END(params_handle); } rb_iv_set(inst, "-exc_status", INT2NUM(exc_status)); diff --git a/test/test_integration_aggregate.rb b/test/test_integration_aggregate.rb index 437ddd23..3c581217 100644 --- a/test/test_integration_aggregate.rb +++ b/test/test_integration_aggregate.rb @@ -427,4 +427,41 @@ def test_step_on_statement_whose_database_was_closed_does_not_use_freed_aggregat values = stmt.step assert_equal 33, values[0] end + + # GHSA-mwm8-39rw-8826: rb_sqlite3_aggregator_step converts the arguments into + # an xcalloc'd VALUE array that is not a GC root. sqlite3val2rb allocates, so a + # GC while converting a later argument could collect a Ruby object already + # stored in an earlier slot and hand the step block a wrong or freed object. + # Needs arity >= 2 (the arity 1 branch uses a pinned stack local). Large column + # values make every conversion allocate, so the window is reachable. + def test_multi_argument_step_arguments_survive_gc + @db.execute("create table wide ( a text, b text )") + filler = "p" * 2000 + @db.transaction do + stmt = @db.prepare("insert into wide values ( ?, ? )") + 200.times { |i| stmt.execute("a-#{i}-#{filler}", "b-#{i}-#{filler}") } + stmt.close + end + + seen = 0 + bad = [] + @db.create_aggregate("checkcols", 2) do + step do |ctx, x, y| + seen += 1 + bad << x unless x.is_a?(String) && x.start_with?("a-") + bad << y unless y.is_a?(String) && y.start_with?("b-") + end + finalize { |ctx| ctx.result = seen } + end + + begin + GC.stress = true + @db.get_first_value("select checkcols(a, b) from wide") + ensure + GC.stress = false + end + + assert_equal 200, seen + assert_empty bad, "aggregate step received corrupted arguments after GC" + end end