Skip to content

THRIFT-6045: Add Ruby recursion depth limit #3691

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kpumuk wants to merge 1 commit into
base: master
Choose a base branch
from
Open

THRIFT-6045: Add Ruby recursion depth limit #3691

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions lib/rb/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,18 @@ certificate must still match the connection hostname.
`Thrift::TransportException::ALREADY_OPEN` when the TCP transport is already
open. Close the transport before opening it again.

Ruby struct, union, and exception serialization methods now accept an optional
remaining struct depth: `read(protocol, depth = 64)` and
`write(protocol, depth = 64)`.
Unknown and mismatched fields remain independently bounded by
`Thrift::BaseProtocol#skip`. The `write_field` and `write_type` helpers accept
an optional `remaining_depth`: custom writers must use
`write_field(field_info, fid, value, remaining_depth)` or
`write_type(field_info, value, remaining_depth)` for struct values. Custom
protocols and custom struct, union, or exception serialization overrides with
exact argument counts must accept and forward the optional depth argument (or
use `*args`) before upgrading.

### 0.24.0

Connect timeout handling changed for both `Thrift::Socket` and
Expand Down
132 changes: 95 additions & 37 deletions lib/rb/ext/struct.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,30 @@ static VALUE default_sym;

#define IS_CONTAINER(ttype) ((ttype) == TTYPE_MAP || (ttype) == TTYPE_LIST || (ttype) == TTYPE_SET)
#define STRUCT_FIELDS(obj) rb_const_get(CLASS_OF(obj), fields_const_id)
// Default budget when callers do not provide a remaining depth.
static int recursion_limit;

static void validate_recursion_depth(int remaining_depth) {
if (RB_UNLIKELY(remaining_depth <= 0)) {
rb_exc_raise(
get_protocol_exception(
INT2FIX(PROTOERR_DEPTH_LIMIT),
rb_str_new2("Maximum recursion depth exceeded")
)
);
}
}

static int parse_recursive_args(int argc, const VALUE *argv, VALUE *protocol) {
if (RB_UNLIKELY(argc < 1 || argc > 2)) {
rb_error_arity(argc, 1, 2);
}

*protocol = argv[0];
int remaining_depth = argc == 1 ? recursion_limit : NUM2INT(argv[1]);
validate_recursion_depth(remaining_depth);
return remaining_depth;
}

static void validate_container_size(int size) {
if (RB_UNLIKELY(size < 0)) {
Expand Down Expand Up @@ -236,9 +260,9 @@ VALUE default_read_struct_end(VALUE protocol) {

// end default protocol methods

static VALUE rb_thrift_union_write (VALUE self, VALUE protocol);
static VALUE rb_thrift_struct_write(VALUE self, VALUE protocol);
static void write_anything(int ttype, VALUE value, VALUE protocol, VALUE field_info);
static VALUE rb_thrift_struct_write_recursive(VALUE self, VALUE protocol, int remaining_depth);
static VALUE rb_thrift_union_write_recursive(VALUE self, VALUE protocol, int remaining_depth);
static void write_anything(int ttype, VALUE value, VALUE protocol, VALUE field_info, int remaining_depth);

static inline ID field_ivar_id(VALUE field_name) {
char name_buf[RSTRING_LEN(field_name) + 2];
Expand All @@ -253,7 +277,7 @@ VALUE get_field_value(VALUE obj, VALUE field_name) {
return rb_ivar_get(obj, field_ivar_id(field_name));
}

static void write_container(int ttype, VALUE field_info, VALUE value, VALUE protocol) {
static void write_container(int ttype, VALUE field_info, VALUE value, VALUE protocol, int remaining_depth) {
long sz, i;

if (ttype == TTYPE_MAP) {
Expand All @@ -280,15 +304,15 @@ static void write_container(int ttype, VALUE field_info, VALUE value, VALUE prot
VALUE val = rb_hash_aref(value, key);

if (IS_CONTAINER(keytype)) {
write_container(keytype, key_info, key, protocol);
write_container(keytype, key_info, key, protocol, remaining_depth);
} else {
write_anything(keytype, key, protocol, key_info);
write_anything(keytype, key, protocol, key_info, remaining_depth);
}

if (IS_CONTAINER(valuetype)) {
write_container(valuetype, value_info, val, protocol);
write_container(valuetype, value_info, val, protocol, remaining_depth);
} else {
write_anything(valuetype, val, protocol, value_info);
write_anything(valuetype, val, protocol, value_info, remaining_depth);
}
}

Expand All @@ -306,9 +330,9 @@ static void write_container(int ttype, VALUE field_info, VALUE value, VALUE prot
for (i = 0; i < sz; ++i) {
VALUE val = rb_ary_entry(value, i);
if (IS_CONTAINER(element_type)) {
write_container(element_type, element_type_info, val, protocol);
write_container(element_type, element_type_info, val, protocol, remaining_depth);
} else {
write_anything(element_type, val, protocol, element_type_info);
write_anything(element_type, val, protocol, element_type_info, remaining_depth);
}
}
default_write_list_end(protocol);
Expand Down Expand Up @@ -337,9 +361,9 @@ static void write_container(int ttype, VALUE field_info, VALUE value, VALUE prot
for (i = 0; i < sz; i++) {
VALUE val = rb_ary_entry(items, i);
if (IS_CONTAINER(element_type)) {
write_container(element_type, element_type_info, val, protocol);
write_container(element_type, element_type_info, val, protocol, remaining_depth);
} else {
write_anything(element_type, val, protocol, element_type_info);
write_anything(element_type, val, protocol, element_type_info, remaining_depth);
}
}

Expand All @@ -349,7 +373,7 @@ static void write_container(int ttype, VALUE field_info, VALUE value, VALUE prot
}
}

static void write_anything(int ttype, VALUE value, VALUE protocol, VALUE field_info) {
static void write_anything(int ttype, VALUE value, VALUE protocol, VALUE field_info, int remaining_depth) {
if (ttype == TTYPE_BOOL) {
default_write_bool(protocol, value);
} else if (ttype == TTYPE_BYTE) {
Expand All @@ -372,19 +396,21 @@ static void write_anything(int ttype, VALUE value, VALUE protocol, VALUE field_i
} else if (ttype == TTYPE_UUID) {
default_write_uuid(protocol, value);
} else if (IS_CONTAINER(ttype)) {
write_container(ttype, field_info, value, protocol);
write_container(ttype, field_info, value, protocol, remaining_depth);
} else if (ttype == TTYPE_STRUCT) {
remaining_depth--;
validate_recursion_depth(remaining_depth);
if (rb_obj_is_kind_of(value, thrift_union_class)) {
rb_thrift_union_write(value, protocol);
rb_thrift_union_write_recursive(value, protocol, remaining_depth);
} else {
rb_thrift_struct_write(value, protocol);
rb_thrift_struct_write_recursive(value, protocol, remaining_depth);
}
} else {
rb_raise(rb_eNotImpError, "Unknown type for binary_encoding: %d", ttype);
}
}

static VALUE rb_thrift_struct_write(VALUE self, VALUE protocol) {
static VALUE rb_thrift_struct_write_recursive(VALUE self, VALUE protocol, int remaining_depth) {
// call validate
rb_funcall(self, validate_method_id, 0);

Expand All @@ -410,7 +436,7 @@ static VALUE rb_thrift_struct_write(VALUE self, VALUE protocol) {
if (!NIL_P(field_value)) {
default_write_field_begin(protocol, field_name, ttype_value, field_id);

write_anything(ttype, field_value, protocol, field_info);
write_anything(ttype, field_value, protocol, field_info, remaining_depth);

default_write_field_end(protocol);
}
Expand All @@ -424,12 +450,19 @@ static VALUE rb_thrift_struct_write(VALUE self, VALUE protocol) {
return Qnil;
}

// cppcheck-suppress constParameterCallback
static VALUE rb_thrift_struct_write(int argc, VALUE *argv, VALUE self) {
VALUE protocol;
int remaining_depth = parse_recursive_args(argc, argv, &protocol);
return rb_thrift_struct_write_recursive(self, protocol, remaining_depth);
}

//-------------------------------------------
// Reading section
//-------------------------------------------

static VALUE rb_thrift_union_read(VALUE self, VALUE protocol);
static VALUE rb_thrift_struct_read(VALUE self, VALUE protocol);
static VALUE rb_thrift_union_read_recursive(VALUE self, VALUE protocol, int remaining_depth);
static VALUE rb_thrift_struct_read_recursive(VALUE self, VALUE protocol, int remaining_depth);
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);

Expand Down Expand Up @@ -490,7 +523,7 @@ static void skip_list_or_set_contents(VALUE protocol, VALUE element_type_value,
}
}

static VALUE read_anything(VALUE protocol, int ttype, VALUE field_info) {
static VALUE read_anything(VALUE protocol, int ttype, VALUE field_info, int remaining_depth) {
VALUE result = Qnil;

if (ttype == TTYPE_BOOL) {
Expand All @@ -517,11 +550,13 @@ static VALUE read_anything(VALUE protocol, int ttype, VALUE field_info) {
} else if (ttype == TTYPE_STRUCT) {
VALUE klass = rb_hash_aref(field_info, class_sym);
result = rb_class_new_instance(0, NULL, klass);
remaining_depth--;
validate_recursion_depth(remaining_depth);

if (rb_obj_is_kind_of(result, thrift_union_class)) {
rb_thrift_union_read(result, protocol);
rb_thrift_union_read_recursive(result, protocol, remaining_depth);
} else {
rb_thrift_struct_read(result, protocol);
rb_thrift_struct_read_recursive(result, protocol, remaining_depth);
}
} else if (ttype == TTYPE_MAP) {
VALUE map_header = default_read_map_begin(protocol);
Expand All @@ -547,8 +582,8 @@ static VALUE read_anything(VALUE protocol, int ttype, VALUE field_info) {
for (int 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, remaining_depth);
val = read_anything(protocol, value_ttype, value_info, remaining_depth);

rb_hash_aset(result, key, val);
}
Expand All @@ -574,7 +609,7 @@ static VALUE read_anything(VALUE protocol, int ttype, VALUE field_info) {
result = new_container_array(num_elements);

for (int i = 0; i < num_elements; ++i) {
rb_ary_push(result, read_anything(protocol, element_ttype, rb_hash_aref(field_info, element_sym)));
rb_ary_push(result, read_anything(protocol, element_ttype, rb_hash_aref(field_info, element_sym), remaining_depth));
}
} else {
validate_container_size(num_elements);
Expand Down Expand Up @@ -602,7 +637,7 @@ static VALUE read_anything(VALUE protocol, int ttype, VALUE field_info) {
items = new_container_array(num_elements);

for (int i = 0; i < num_elements; ++i) {
rb_ary_push(items, read_anything(protocol, element_ttype, rb_hash_aref(field_info, element_sym)));
rb_ary_push(items, read_anything(protocol, element_ttype, rb_hash_aref(field_info, element_sym), remaining_depth));
}

result = rb_class_new_instance(1, &items, rb_cSet);
Expand All @@ -623,7 +658,7 @@ static VALUE read_anything(VALUE protocol, int ttype, VALUE field_info) {
return result;
}

static VALUE rb_thrift_struct_read(VALUE self, VALUE protocol) {
static VALUE rb_thrift_struct_read_recursive(VALUE self, VALUE protocol, int remaining_depth) {
VALUE struct_fields = STRUCT_FIELDS(self);

if (RHASH_SIZE(struct_fields) > 0 && rb_ivar_count(self) > 0) {
Expand Down Expand Up @@ -652,7 +687,7 @@ static VALUE rb_thrift_struct_read(VALUE self, VALUE protocol) {
if (field_type == specified_type) {
// read the value
VALUE name = rb_hash_aref(field_info, name_sym);
set_field_value(self, name, read_anything(protocol, field_type, field_info));
set_field_value(self, name, read_anything(protocol, field_type, field_info, remaining_depth));
} else {
rb_funcall(protocol, skip_method_id, 1, field_type_value);
}
Expand All @@ -673,12 +708,19 @@ static VALUE rb_thrift_struct_read(VALUE self, VALUE protocol) {
return Qnil;
}

// cppcheck-suppress constParameterCallback
static VALUE rb_thrift_struct_read(int argc, VALUE *argv, VALUE self) {
VALUE protocol;
int remaining_depth = parse_recursive_args(argc, argv, &protocol);
return rb_thrift_struct_read_recursive(self, protocol, remaining_depth);
}


// --------------------------------
// Union section
// --------------------------------

static VALUE rb_thrift_union_read(VALUE self, VALUE protocol) {
static VALUE rb_thrift_union_read_recursive(VALUE self, VALUE protocol, int remaining_depth) {
rb_check_frozen(self);

if (!NIL_P(rb_ivar_get(self, setfield_id))) {
Expand All @@ -705,7 +747,7 @@ static VALUE rb_thrift_union_read(VALUE self, VALUE protocol) {
if (field_type == specified_type) {
// read the value
VALUE name = rb_hash_aref(field_info, name_sym);
VALUE value = read_anything(protocol, field_type, field_info);
VALUE value = read_anything(protocol, field_type, field_info, remaining_depth);
rb_iv_set(self, "@setfield", rb_str_intern(name));
rb_iv_set(self, "@value", value);
} else {
Expand Down Expand Up @@ -735,7 +777,14 @@ static VALUE rb_thrift_union_read(VALUE self, VALUE protocol) {
return Qnil;
}

static VALUE rb_thrift_union_write(VALUE self, VALUE protocol) {
// cppcheck-suppress constParameterCallback
static VALUE rb_thrift_union_read(int argc, VALUE *argv, VALUE self) {
VALUE protocol;
int remaining_depth = parse_recursive_args(argc, argv, &protocol);
return rb_thrift_union_read_recursive(self, protocol, remaining_depth);
}

static VALUE rb_thrift_union_write_recursive(VALUE self, VALUE protocol, int remaining_depth) {
// call validate
rb_funcall(self, validate_method_id, 0);

Expand All @@ -759,7 +808,7 @@ static VALUE rb_thrift_union_write(VALUE self, VALUE protocol) {

default_write_field_begin(protocol, setfield, ttype_value, field_id);

write_anything(ttype, setvalue, protocol, field_info);
write_anything(ttype, setvalue, protocol, field_info, remaining_depth);

default_write_field_end(protocol);

Expand All @@ -771,17 +820,26 @@ static VALUE rb_thrift_union_write(VALUE self, VALUE protocol) {
return Qnil;
}

// cppcheck-suppress constParameterCallback
static VALUE rb_thrift_union_write(int argc, VALUE *argv, VALUE self) {
VALUE protocol;
int remaining_depth = parse_recursive_args(argc, argv, &protocol);
return rb_thrift_union_write_recursive(self, protocol, remaining_depth);
}

void Init_struct(void) {
VALUE struct_module = rb_const_get(thrift_module, rb_intern("Struct"));

rb_define_method(struct_module, "write", rb_thrift_struct_write, 1);
rb_define_method(struct_module, "read", rb_thrift_struct_read, 1);
recursion_limit = FIX2INT(rb_const_get(thrift_module, rb_intern("DEFAULT_RECURSION_DEPTH")));

rb_define_method(struct_module, "write", rb_thrift_struct_write, -1);
rb_define_method(struct_module, "read", rb_thrift_struct_read, -1);
Comment thread
kpumuk marked this conversation as resolved.

thrift_union_class = rb_const_get(thrift_module, rb_intern("Union"));
rb_global_variable(&thrift_union_class);

rb_define_method(thrift_union_class, "write", rb_thrift_union_write, 1);
rb_define_method(thrift_union_class, "read", rb_thrift_union_read, 1);
rb_define_method(thrift_union_class, "write", rb_thrift_union_write, -1);
rb_define_method(thrift_union_class, "read", rb_thrift_union_read, -1);

setfield_id = rb_intern("@setfield");
rb_global_variable(&setfield_id);
Expand Down
8 changes: 6 additions & 2 deletions lib/rb/lib/thrift/exceptions.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
#

module Thrift
DEFAULT_RECURSION_DEPTH = 64

class Exception < StandardError
def initialize(message)
super
Expand Down Expand Up @@ -49,7 +51,8 @@ def initialize(type = UNKNOWN, message = nil)
@type = type
end

def read(iprot)
def read(iprot, remaining_depth = DEFAULT_RECURSION_DEPTH)
raise ProtocolException.new(ProtocolException::DEPTH_LIMIT, 'Maximum recursion depth exceeded') if remaining_depth <= 0
iprot.read_struct_begin
while true
fname, ftype, fid = iprot.read_field_begin
Expand All @@ -68,7 +71,8 @@ def read(iprot)
iprot.read_struct_end
end

def write(oprot)
def write(oprot, remaining_depth = DEFAULT_RECURSION_DEPTH)
raise ProtocolException.new(ProtocolException::DEPTH_LIMIT, 'Maximum recursion depth exceeded') if remaining_depth <= 0
oprot.write_struct_begin('Thrift::ApplicationException')
unless @message.nil?
oprot.write_field_begin('message', Types::STRING, 1)
Expand Down
Loading
Loading