From 01ae5922b71853d6cd67d5d0da54feca1e0d84fc Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Thu, 7 Mar 2024 09:42:18 -0500 Subject: [PATCH] Store valid Ruby fixnum into array The filter creates a Ruby array, but the argc is an unsigned long rather than a Ruby fixnum. This crashes when running the included test with GC stress (by setting the GC_STRESS environment variable). --- ext/liquid_c/liquid_vm.c | 8 ++++---- ext/liquid_c/vm_assembler.c | 2 +- test/unit/variable_test.rb | 3 +++ 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/ext/liquid_c/liquid_vm.c b/ext/liquid_c/liquid_vm.c index 36bc2949..9e36c136 100644 --- a/ext/liquid_c/liquid_vm.c +++ b/ext/liquid_c/liquid_vm.c @@ -166,7 +166,7 @@ static inline void vm_stack_reserve_for_write(vm_t *vm, size_t num_values) c_buffer_reserve_for_write(&vm->stack, num_values * sizeof(VALUE)); } -static VALUE vm_invoke_filter(vm_t *vm, VALUE filter_name, int num_args) +static VALUE vm_invoke_filter(vm_t *vm, VALUE filter_name, size_t num_args) { VALUE *popped_args = vm_stack_pop_n(vm, num_args); /* We have to copy popped_args_ptr to the stack because the VM @@ -185,7 +185,7 @@ static VALUE vm_invoke_filter(vm_t *vm, VALUE filter_name, int num_args) } vm->invoking_filter = true; - VALUE result = rb_funcallv(vm->context.strainer, RB_SYM2ID(filter_name), num_args, args); + VALUE result = rb_funcallv(vm->context.strainer, RB_SYM2ID(filter_name), (int)num_args, args); vm->invoking_filter = false; return rb_funcall(result, id_to_liquid, 0); } @@ -334,13 +334,13 @@ static VALUE vm_render_until_error(VALUE uncast_args) case OP_BUILTIN_FILTER: { VALUE filter_name; - uint8_t num_args; + unsigned long num_args; if (ip[-1] == OP_FILTER) { constant_index = (ip[0] << 8) | ip[1]; constant = constants[constant_index]; filter_name = RARRAY_AREF(constant, 0); - num_args = RARRAY_AREF(constant, 1); + num_args = FIX2ULONG(RARRAY_AREF(constant, 1)); ip += 2; } else { assert(ip[-1] == OP_BUILTIN_FILTER); diff --git a/ext/liquid_c/vm_assembler.c b/ext/liquid_c/vm_assembler.c index 7dd96f16..f48e789b 100644 --- a/ext/liquid_c/vm_assembler.c +++ b/ext/liquid_c/vm_assembler.c @@ -362,7 +362,7 @@ void vm_assembler_add_filter(vm_assembler_t *code, VALUE filter_name, size_t arg } else { VALUE filter_args = rb_ary_new_capa(2); rb_ary_push(filter_args, filter_name); - rb_ary_push(filter_args, arg_count + 1); + rb_ary_push(filter_args, LONG2FIX((long)(arg_count + 1))); vm_assembler_add_op_with_constant(code, filter_args, OP_FILTER); } } diff --git a/test/unit/variable_test.rb b/test/unit/variable_test.rb index beab9cf6..aa0bb14e 100644 --- a/test/unit/variable_test.rb +++ b/test/unit/variable_test.rb @@ -80,6 +80,9 @@ def test_variable_filter_args output = variable_strict_parse("name | filter1 : a , b : c , d : e").render!(context, render_opts) assert_equal('{ filter: :filter1, input: "Bob", args: [1, {"b"=>3, "d"=>5}] }', output) + output = variable_strict_parse("name | filter1: 1, 2, 3, 4, 5, 6, 7").render!(context, render_opts) + assert_equal('{ filter: :filter1, input: "Bob", args: [1, 2, 3, 4, 5, 6, 7] }', output) + assert_raises(Liquid::SyntaxError) do variable_strict_parse("name | filter : a : b : c : d : e") end