Skip to content
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.

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

WIP: Prevent segfaults related to infinite marshal recursion #198

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,4 @@ Gemfile.lock
lib/mini_racer_extension.so
lib/mini_racer_loader.so
*.bundle
.vscode/
86 changes: 85 additions & 1 deletion ext/mini_racer_extension/mini_racer_extension.cc
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,71 @@ enum IsolateFlags {
DO_TERMINATE,
MEM_SOFTLIMIT_VALUE,
MEM_SOFTLIMIT_REACHED,
MARSHAL_STACKDEPTH_VALUE,
MARSHAL_STACKDEPTH_REACHED,
};

struct StackCounter {
#define MARSHAL_STACKDEPTH_MAX 1000

static void Reset(Isolate* isolate) {
if (MARSHAL_STACKDEPTH_MAX > 0) {
isolate->SetData(MARSHAL_STACKDEPTH_VALUE, (void*)0);
isolate->SetData(MARSHAL_STACKDEPTH_REACHED, (void*)false);
}
}

StackCounter(Isolate* isolate) {
this->isActive = MARSHAL_STACKDEPTH_MAX > 0;

if (this->isActive) {
this->isolate = isolate;
this->IncDepth(1);
}
}

bool IsTooDeep() {
if (!this->IsActive()) {
return false;
}

size_t depth = (size_t)this->isolate->GetData(MARSHAL_STACKDEPTH_VALUE);
size_t maxDepth = MARSHAL_STACKDEPTH_MAX;
if (depth > maxDepth) {
printf("setting maxdepth reached flag\n");
this->isolate->SetData(MARSHAL_STACKDEPTH_REACHED, (void*)true);
return true;
}

return false;
}

bool IsActive() {
return this->isActive && !(bool)this->isolate->GetData(DO_TERMINATE);
}

~StackCounter() {
if (this->IsActive()) {
this->IncDepth(-1);
}
}

private:
Isolate* isolate;
bool isActive;

void IncDepth(int direction) {
int inc = direction > 0 ? 1 : -1;

size_t depth = (size_t)this->isolate->GetData(MARSHAL_STACKDEPTH_VALUE);

// don't decrement past 0
if (inc > 0 || depth > 0) {
depth += inc;
}
// printf("%zu\n", depth);
this->isolate->SetData(MARSHAL_STACKDEPTH_VALUE, (void*)depth);
}
};

static VALUE rb_cContext;
Expand Down Expand Up @@ -335,6 +400,9 @@ nogvl_context_eval(void* arg) {
// Memory softlimit hit flag
isolate->SetData(MEM_SOFTLIMIT_REACHED, (void*)false);

isolate->SetData(MARSHAL_STACKDEPTH_VALUE, (void*)false);
isolate->SetData(MARSHAL_STACKDEPTH_REACHED, (void*)false);

MaybeLocal<Script> parsed_script;

if (eval_params->filename) {
Expand Down Expand Up @@ -390,6 +458,14 @@ static VALUE convert_v8_to_ruby(Isolate* isolate, Local<Context> context,
Isolate::Scope isolate_scope(isolate);
HandleScope scope(isolate);

StackCounter stackCounter(isolate);

if (stackCounter.IsTooDeep()) {
isolate->SetData(DO_TERMINATE, (void*)true);
isolate->TerminateExecution();
return Qnil;
}

if (value->IsNull() || value->IsUndefined()){
return Qnil;
}
Expand Down Expand Up @@ -885,8 +961,14 @@ static VALUE convert_result_to_ruby(VALUE self /* context */,
VALUE ruby_exception = rb_iv_get(self, "@current_exception");
if (ruby_exception == Qnil) {
bool mem_softlimit_reached = (bool)isolate->GetData(MEM_SOFTLIMIT_REACHED);
bool marshal_stack_maxdepth_reached = (bool)isolate->GetData(MARSHAL_STACKDEPTH_REACHED);
printf("stackdepth reached? %p; %d < %d = %s\n", isolate->GetData(MARSHAL_STACKDEPTH_REACHED), MARSHAL_STACKDEPTH_REACHED, isolate->GetNumberOfDataSlots(), MARSHAL_STACKDEPTH_REACHED < isolate->GetNumberOfDataSlots() ? "true" : "false");
// If we were terminated or have the memory softlimit flag set
if (result.terminated || mem_softlimit_reached) {
if (marshal_stack_maxdepth_reached) {
ruby_exception = rb_eScriptRuntimeError;
std::string msg = std::string("Marshal object depth too deep. Script terminated.");
message = rb_enc_str_new(msg.c_str(), msg.length(), rb_enc_find("utf-8"));
} else if (result.terminated || mem_softlimit_reached) {
ruby_exception = mem_softlimit_reached ? rb_eV8OutOfMemoryError : rb_eScriptTerminatedError;
} else {
ruby_exception = rb_eScriptRuntimeError;
Expand Down Expand Up @@ -921,6 +1003,7 @@ static VALUE convert_result_to_ruby(VALUE self /* context */,
VALUE json_string = rb_enc_str_new(*String::Utf8Value(isolate, rstr), rstr->Utf8Length(isolate), rb_enc_find("utf-8"));
ret = rb_funcall(rb_mJSON, rb_intern("parse"), 1, json_string);
} else {
StackCounter::Reset(isolate);
ret = convert_v8_to_ruby(isolate, *p_ctx, tmp);
}

Expand Down Expand Up @@ -1057,6 +1140,7 @@ gvl_ruby_callback(void* data) {

for (int i = 0; i < length; i++) {
Local<Value> value = ((*args)[i]).As<Value>();
StackCounter::Reset(args->GetIsolate());
VALUE tmp = convert_v8_to_ruby(args->GetIsolate(),
*context_info->context, value);
rb_ary_push(ruby_args, tmp);
Expand Down
40 changes: 40 additions & 0 deletions test/mini_racer_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -864,6 +864,46 @@ def test_symbol_support
assert_equal :foo, context.eval("Symbol('foo')")
end

def test_cyclical_object_js
context = MiniRacer::Context.new()
context.attach("a", proc{|a| a})

assert_raises(MiniRacer::RuntimeError) { context.eval("let arr = []; arr.push(arr); a(arr)") }
end

def test_infinite_object_js
context = MiniRacer::Context.new()
context.attach("a", proc{|a| a})

js = <<~JS
var d=0;
function get(z) {
z.depth=d++; // this isn't necessary to make it infinite, just to make it more obvious that it is
Object.defineProperty(z,'foo',{get(){var r={};return get(r);},enumerable:true})
return z;
}
a(get({}));
JS

assert_raises(MiniRacer::RuntimeError) { context.eval(js) }
end

def test_deep_object_js
context = MiniRacer::Context.new()
context.attach("a", proc{|a| a})

# stack depth should be enough to marshal the object
assert_equal [[[]]], context.eval("let arr = [[[]]]; a(arr)")
end

def test_too_deep_object_js
context = MiniRacer::Context.new()
context.attach("a", proc{|a| a})

# too deep
assert_raises(MiniRacer::RuntimeError) { context.eval("let arr = [[[[[[[[]]]]]]]]; a(arr)") }
end

def test_proxy_support
js = <<~JS
function MyProxy(reference) {
Expand Down