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 db78efa..90c4b05 100644
--- a/lib/rb/ext/struct.c
+++ b/lib/rb/ext/struct.c
@@ -414,6 +414,8 @@
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 @@
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 @@
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 @@
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 @@
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 9bfbed5..849c856 100644
--- a/lib/rb/lib/thrift/struct_union.rb
+++ b/lib/rb/lib/thrift/struct_union.rb
@@ -46,25 +46,49 @@
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