From 7f14ea9f48f21ce3c1528da56a3f101492324f45 Mon Sep 17 00:00:00 2001 From: Joe Bolinger Date: Wed, 27 Feb 2019 20:26:14 -0800 Subject: [PATCH] Raise error for JSON overflow encoding in Ruby (#5752) * add check for overflow * de-nestify * break long lines --- ruby/Rakefile | 4 +- ruby/ext/google/protobuf_c/encode_decode.c | 5 ++ ruby/tests/basic_test.proto | 14 ++++- ruby/tests/basic_test_proto2.proto | 11 ++++ ruby/tests/common_tests.rb | 61 ++++++++++++++++++++++ 5 files changed, 92 insertions(+), 3 deletions(-) diff --git a/ruby/Rakefile b/ruby/Rakefile index 79b7df3065..140f5e5149 100644 --- a/ruby/Rakefile +++ b/ruby/Rakefile @@ -118,11 +118,11 @@ file "tests/test_ruby_package_proto2.rb" => "tests/test_ruby_package_proto2.prot end file "tests/basic_test.rb" => "tests/basic_test.proto" do |file_task| - sh "../src/protoc --ruby_out=. tests/basic_test.proto" + sh "../src/protoc -I../src -I. --ruby_out=. tests/basic_test.proto" end file "tests/basic_test_proto2.rb" => "tests/basic_test_proto2.proto" do |file_task| - sh "../src/protoc --ruby_out=. tests/basic_test_proto2.proto" + sh "../src/protoc -I../src -I. --ruby_out=. tests/basic_test_proto2.proto" end task :genproto => genproto_output diff --git a/ruby/ext/google/protobuf_c/encode_decode.c b/ruby/ext/google/protobuf_c/encode_decode.c index c5078bcb73..5ead9b8984 100644 --- a/ruby/ext/google/protobuf_c/encode_decode.c +++ b/ruby/ext/google/protobuf_c/encode_decode.c @@ -1061,6 +1061,11 @@ static void put_ruby_value(VALUE value, upb_sink *sink, bool emit_defaults, bool is_json) { + if (depth > ENCODE_MAX_NESTING) { + rb_raise(rb_eRuntimeError, + "Maximum recursion depth exceeded during encoding."); + } + upb_selector_t sel = 0; if (upb_fielddef_isprimitive(f)) { sel = getsel(f, upb_handlers_getprimitivehandlertype(f)); diff --git a/ruby/tests/basic_test.proto b/ruby/tests/basic_test.proto index 4010fe30d0..684f3e6202 100644 --- a/ruby/tests/basic_test.proto +++ b/ruby/tests/basic_test.proto @@ -2,6 +2,8 @@ syntax = "proto3"; package basic_test; +import "google/protobuf/struct.proto"; + message Foo { Bar bar = 1; repeated Baz baz = 2; @@ -106,4 +108,14 @@ message Outer { } message Inner { -} \ No newline at end of file +} + + +message MyRepeatedStruct { + repeated MyStruct structs = 1; +} + +message MyStruct { + string string = 1; + google.protobuf.Struct struct = 2; +} diff --git a/ruby/tests/basic_test_proto2.proto b/ruby/tests/basic_test_proto2.proto index a503eae635..2c4b300f9c 100644 --- a/ruby/tests/basic_test_proto2.proto +++ b/ruby/tests/basic_test_proto2.proto @@ -2,6 +2,8 @@ syntax = "proto2"; package basic_test_proto2; +import "google/protobuf/struct.proto"; + message Foo { optional Bar bar = 1; repeated Baz baz = 2; @@ -115,3 +117,12 @@ message OneofMessage { TestEnum d = 4; } } + +message MyRepeatedStruct { + repeated MyStruct structs = 1; +} + +message MyStruct { + optional string string = 1; + optional google.protobuf.Struct struct = 2; +} diff --git a/ruby/tests/common_tests.rb b/ruby/tests/common_tests.rb index 38763001c9..9464deb046 100644 --- a/ruby/tests/common_tests.rb +++ b/ruby/tests/common_tests.rb @@ -1137,6 +1137,67 @@ module CommonTests assert JSON.parse(actual, :symbolize_names => true) == expected end + def value_from_ruby(value) + ret = Google::Protobuf::Value.new + case value + when String + ret.string_value = value + when Google::Protobuf::Struct + ret.struct_value = value + when Hash + ret.struct_value = struct_from_ruby(value) + when Google::Protobuf::ListValue + ret.list_value = value + when Array + ret.list_value = list_from_ruby(value) + else + @log.error "Unknown type: #{value.class}" + raise Google::Protobuf::Error, "Unknown type: #{value.class}" + end + ret + end + + def list_from_ruby(arr) + ret = Google::Protobuf::ListValue.new + arr.each do |v| + ret.values << value_from_ruby(v) + end + ret + end + + def struct_from_ruby(hash) + ret = Google::Protobuf::Struct.new + hash.each do |k, v| + ret.fields[k] ||= value_from_ruby(v) + end + ret + end + + def test_deep_json + # will not overflow + json = '{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":'\ + '{"a":{"a":{"a":{"a":{}}}}}}}}}}}}}}}}' + + struct = struct_from_ruby(JSON.parse(json)) + assert_equal json, struct.to_json + + encoded = proto_module::MyRepeatedStruct.encode( + proto_module::MyRepeatedStruct.new(structs: [proto_module::MyStruct.new(struct: struct)])) + assert_equal json, proto_module::MyRepeatedStruct.decode(encoded).structs[0].struct.to_json + + # will overflow + json = '{"a":{"a":{"a":[{"a":{"a":[{"a":[{"a":{"a":[{"a":[{"a":'\ + '{"a":[{"a":[{"a":{"a":{"a":[{"a":"a"}]}}}]}]}}]}]}}]}]}}]}}}' + + struct = struct_from_ruby(JSON.parse(json)) + assert_equal json, struct.to_json + + assert_raise(RuntimeError, "Maximum recursion depth exceeded during encoding") do + proto_module::MyRepeatedStruct.encode( + proto_module::MyRepeatedStruct.new(structs: [proto_module::MyStruct.new(struct: struct)])) + end + end + def test_comparison_with_arbitrary_object assert proto_module::TestMessage.new != nil end