From: Bryan Duxbury Date: Tue, 31 May 2011 20:03:29 +0000 (+0000) Subject: THRIFT-1182. rb: Native deserializer segfaults on incorrect list element type X-Git-Tag: 0.7.0~88 X-Git-Url: https://source.supwisdom.com/gerrit/gitweb?a=commitdiff_plain;h=911d2f1576197fbc4cc0f6b98c0fd5998400f906;p=common%2Fthrift.git THRIFT-1182. rb: Native deserializer segfaults on incorrect list element type This patch causes both the pure ruby and native extension code paths to check if the data in lists, sets, and maps is of the expected type before deserlizing it. When it's not the right type, it now skips the bad data correctly. Patch: Ilya Maykov git-svn-id: https://svn.apache.org/repos/asf/thrift/trunk@1129892 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/lib/rb/ext/struct.c b/lib/rb/ext/struct.c index db78efa8..90c4b05a 100644 --- a/lib/rb/ext/struct.c +++ b/lib/rb/ext/struct.c @@ -414,6 +414,8 @@ static VALUE rb_thrift_struct_write(VALUE self, VALUE protocol) { static VALUE rb_thrift_union_read(VALUE self, VALUE protocol); static VALUE rb_thrift_struct_read(VALUE self, VALUE protocol); +static void skip_map_contents(VALUE protocol, VALUE key_type_value, VALUE value_type_value, int size); +static void skip_list_or_set_contents(VALUE protocol, VALUE element_type_value, int size); static void set_field_value(VALUE obj, VALUE field_name, VALUE value) { char name_buf[RSTRING_LEN(field_name) + 1]; @@ -424,6 +426,23 @@ static void set_field_value(VALUE obj, VALUE field_name, VALUE value) { rb_ivar_set(obj, rb_intern(name_buf), value); } +// Helper method to skip the contents of a map (assumes the map header has been read). +static void skip_map_contents(VALUE protocol, VALUE key_type_value, VALUE value_type_value, int size) { + int i; + for (i = 0; i < size; i++) { + rb_funcall(protocol, skip_method_id, 1, key_type_value); + rb_funcall(protocol, skip_method_id, 1, value_type_value); + } +} + +// Helper method to skip the contents of a list or set (assumes the list/set header has been read). +static void skip_list_or_set_contents(VALUE protocol, VALUE element_type_value, int size) { + int i; + for (i = 0; i < size; i++) { + rb_funcall(protocol, skip_method_id, 1, element_type_value); + } +} + static VALUE read_anything(VALUE protocol, int ttype, VALUE field_info) { VALUE result = Qnil; @@ -458,18 +477,30 @@ static VALUE read_anything(VALUE protocol, int ttype, VALUE field_info) { int value_ttype = FIX2INT(rb_ary_entry(map_header, 1)); int num_entries = FIX2INT(rb_ary_entry(map_header, 2)); + // Check the declared key and value types against the expected ones and skip the map contents + // if the types don't match. VALUE key_info = rb_hash_aref(field_info, key_sym); VALUE value_info = rb_hash_aref(field_info, value_sym); - result = rb_hash_new(); + if (!NIL_P(key_info) && !NIL_P(value_info)) { + int specified_key_type = FIX2INT(rb_hash_aref(key_info, type_sym)); + int specified_value_type = FIX2INT(rb_hash_aref(value_info, type_sym)); + if (specified_key_type == key_ttype && specified_value_type == value_ttype) { + result = rb_hash_new(); - for (i = 0; i < num_entries; ++i) { - VALUE key, val; + for (i = 0; i < num_entries; ++i) { + VALUE key, val; - key = read_anything(protocol, key_ttype, key_info); - val = read_anything(protocol, value_ttype, value_info); + key = read_anything(protocol, key_ttype, key_info); + val = read_anything(protocol, value_ttype, value_info); - rb_hash_aset(result, key, val); + rb_hash_aset(result, key, val); + } + } else { + skip_map_contents(protocol, INT2FIX(key_ttype), INT2FIX(value_ttype), num_entries); + } + } else { + skip_map_contents(protocol, INT2FIX(key_ttype), INT2FIX(value_ttype), num_entries); } default_read_map_end(protocol); @@ -479,10 +510,23 @@ static VALUE read_anything(VALUE protocol, int ttype, VALUE field_info) { VALUE list_header = default_read_list_begin(protocol); int element_ttype = FIX2INT(rb_ary_entry(list_header, 0)); int num_elements = FIX2INT(rb_ary_entry(list_header, 1)); - result = rb_ary_new2(num_elements); - for (i = 0; i < num_elements; ++i) { - rb_ary_push(result, read_anything(protocol, element_ttype, rb_hash_aref(field_info, element_sym))); + // Check the declared element type against the expected one and skip the list contents + // if the types don't match. + VALUE element_info = rb_hash_aref(field_info, element_sym); + if (!NIL_P(element_info)) { + int specified_element_type = FIX2INT(rb_hash_aref(element_info, type_sym)); + if (specified_element_type == element_ttype) { + result = rb_ary_new2(num_elements); + + for (i = 0; i < num_elements; ++i) { + rb_ary_push(result, read_anything(protocol, element_ttype, rb_hash_aref(field_info, element_sym))); + } + } else { + skip_list_or_set_contents(protocol, INT2FIX(element_ttype), num_elements); + } + } else { + skip_list_or_set_contents(protocol, INT2FIX(element_ttype), num_elements); } default_read_list_end(protocol); @@ -493,15 +537,28 @@ static VALUE read_anything(VALUE protocol, int ttype, VALUE field_info) { VALUE set_header = default_read_set_begin(protocol); int element_ttype = FIX2INT(rb_ary_entry(set_header, 0)); int num_elements = FIX2INT(rb_ary_entry(set_header, 1)); - items = rb_ary_new2(num_elements); - for (i = 0; i < num_elements; ++i) { - rb_ary_push(items, read_anything(protocol, element_ttype, rb_hash_aref(field_info, element_sym))); + // Check the declared element type against the expected one and skip the set contents + // if the types don't match. + VALUE element_info = rb_hash_aref(field_info, element_sym); + if (!NIL_P(element_info)) { + int specified_element_type = FIX2INT(rb_hash_aref(element_info, type_sym)); + if (specified_element_type == element_ttype) { + items = rb_ary_new2(num_elements); + + for (i = 0; i < num_elements; ++i) { + rb_ary_push(items, read_anything(protocol, element_ttype, rb_hash_aref(field_info, element_sym))); + } + + result = rb_class_new_instance(1, &items, rb_cSet); + } else { + skip_list_or_set_contents(protocol, INT2FIX(element_ttype), num_elements); + } + } else { + skip_list_or_set_contents(protocol, INT2FIX(element_ttype), num_elements); } default_read_set_end(protocol); - - result = rb_class_new_instance(1, &items, rb_cSet); } else { rb_raise(rb_eNotImpError, "read_anything not implemented for type %d!", ttype); } diff --git a/lib/rb/lib/thrift/struct_union.rb b/lib/rb/lib/thrift/struct_union.rb index 9bfbed5a..849c856d 100644 --- a/lib/rb/lib/thrift/struct_union.rb +++ b/lib/rb/lib/thrift/struct_union.rb @@ -46,25 +46,49 @@ module Thrift value.read(iprot) when Types::MAP key_type, val_type, size = iprot.read_map_begin - value = {} - size.times do - k = read_field(iprot, field_info(field[:key])) - v = read_field(iprot, field_info(field[:value])) - value[k] = v + # Skip the map contents if the declared key or value types don't match the expected ones. + if (key_type != field[:key][:type] || val_type != field[:value][:type]) + size.times do + iprot.skip(key_type) + iprot.skip(val_type) + end + value = nil + else + value = {} + size.times do + k = read_field(iprot, field_info(field[:key])) + v = read_field(iprot, field_info(field[:value])) + value[k] = v + end end iprot.read_map_end when Types::LIST e_type, size = iprot.read_list_begin - value = Array.new(size) do |n| - read_field(iprot, field_info(field[:element])) + # Skip the list contents if the declared element type doesn't match the expected one. + if (e_type != field[:element][:type]) + size.times do + iprot.skip(e_type) + end + value = nil + else + value = Array.new(size) do |n| + read_field(iprot, field_info(field[:element])) + end end iprot.read_list_end when Types::SET e_type, size = iprot.read_set_begin - value = Set.new - size.times do - element = read_field(iprot, field_info(field[:element])) - value << element + # Skip the set contents if the declared element type doesn't match the expected one. + if (e_type != field[:element][:type]) + size.times do + iprot.skip(e_type) + end + else + value = Set.new + size.times do + element = read_field(iprot, field_info(field[:element])) + value << element + end end iprot.read_set_end else