Skip to content

Commit 672da19

Browse files
ydahbyroot
authored andcommitted
Prevent crashes with non-Proc on_load callbacks
Convert non-Proc on_load callbacks through to_proc and validate the result in the shared parser configuration. Previously, arbitrary values were stored and passed directly to rb_proc_call_with_block. Cover Method callbacks in JSON.parse, JSON::Coder, and ResumableParser, along with invalid types, invalid conversion results, and nil or false callbacks.
1 parent b6a5d35 commit 672da19

4 files changed

Lines changed: 53 additions & 1 deletion

File tree

‎ext/json/ext/parser/parser.c‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2021,7 +2021,15 @@ static int parser_config_init_i(VALUE key, VALUE val, VALUE data)
20212021
else if (key == sym_allow_invalid_escape) { config->allow_invalid_escape = RTEST(val); }
20222022
else if (key == sym_symbolize_names) { config->symbolize_names = RTEST(val); }
20232023
else if (key == sym_freeze) { config->freeze = RTEST(val); }
2024-
else if (key == sym_on_load) { parser_config_wb_write(self, &config->on_load_proc, RTEST(val) ? val : Qfalse); }
2024+
else if (key == sym_on_load) {
2025+
if (RTEST(val) && !rb_obj_is_proc(val)) {
2026+
val = rb_check_funcall(val, rb_intern("to_proc"), 0, NULL);
2027+
if (val == Qundef || !rb_obj_is_proc(val)) {
2028+
rb_raise(rb_eTypeError, "on_load must be a Proc");
2029+
}
2030+
}
2031+
parser_config_wb_write(self, &config->on_load_proc, RTEST(val) ? val : Qfalse);
2032+
}
20252033
else if (key == sym_allow_duplicate_key) { config->allow_duplicate_key = RTEST(val); }
20262034
else if (key == sym_decimal_class) {
20272035
if (RTEST(val)) {

‎test/json/json_coder_test.rb‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,16 @@ def test_json_coder_load_options
5656
assert_equal({a: 1}, coder.load('{"a":1}'))
5757
end
5858

59+
def test_json_coder_load_with_on_load_method
60+
on_load = ->(value) { Integer === value ? value + 1 : value }.method(:call)
61+
coder = JSON::Coder.new(on_load: on_load)
62+
assert_equal [2], coder.load('[1]')
63+
end
64+
65+
def test_json_coder_on_load_invalid_type
66+
assert_raise(TypeError) { JSON::Coder.new(on_load: 'x') }
67+
end
68+
5969
def test_json_coder_dump_NaN_or_Infinity
6070
coder = JSON::Coder.new { |o| o.inspect }
6171
assert_equal "NaN", coder.load(coder.dump(Float::NAN))

‎test/json/json_parser_test.rb‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,32 @@ def test_parsing
3838
assert_equal 'test', parser.parse
3939
end
4040

41+
def test_on_load_method
42+
on_load = ->(value) { Integer === value ? value + 1 : value }.method(:call)
43+
assert_equal [2], JSON.parse('[1]', on_load: on_load)
44+
end
45+
46+
def test_on_load_invalid_type
47+
['x', Object.new, Time.now].each do |on_load|
48+
assert_raise(TypeError) { JSON.parse('[1]', on_load: on_load) }
49+
end
50+
end
51+
52+
def test_on_load_invalid_to_proc
53+
on_load = Object.new
54+
def on_load.to_proc
55+
method(:to_proc)
56+
end
57+
assert_raise(TypeError) { JSON.parse('[1]', on_load: on_load) }
58+
end
59+
60+
def test_on_load_falsy
61+
[nil, false].each do |on_load|
62+
config = JSON::Parser::Config.new(on_load: on_load)
63+
assert_equal [1], config.parse('[1]')
64+
end
65+
end
66+
4167
def test_parser_reset
4268
parser = Parser.new('{"a":"b"}')
4369
assert_equal({ 'a' => 'b' }, parser.parse)

‎test/json/resumable_parser_test.rb‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,14 @@ def test_nested_parse_error
106106
end
107107
end
108108

109+
def test_on_load_method
110+
on_load = ->(value) { Integer === value ? value + 1 : value }.method(:call)
111+
parser = new_parser(on_load: on_load)
112+
parser << '[1]'
113+
assert parser.parse
114+
assert_equal [2], parser.value
115+
end
116+
109117
def test_parse_document_direct
110118
@parser << '[true]'
111119
assert_equal true, @parser.parse

0 commit comments

Comments
 (0)