From 5274ab4c3071d1a986cc0036f1696ed5fff9f1a8 Mon Sep 17 00:00:00 2001
From: seanmakesgames <sean@drizzlybear.com>
Date: Wed, 12 May 2021 23:08:22 -0700
Subject: [PATCH] FEATURE: Configurable max marshal stack depth (#202)

`marshal_stack_depth` can be used to determine how deep objects are allowed to be when marshalling JS to Ruby objects in evals.
---
 .gitignore                                    |   1 +
 README.md                                     |  13 ++
 .../mini_racer_extension.cc                   | 152 ++++++++++++++++--
 lib/mini_racer.rb                             |  13 +-
 test/mini_racer_test.rb                       |  61 ++++++-
 5 files changed, 222 insertions(+), 18 deletions(-)

diff --git a/.gitignore b/.gitignore
index a13ff12..8a73970 100644
--- a/.gitignore
+++ b/.gitignore
@@ -10,3 +10,4 @@ Gemfile.lock
 lib/mini_racer_extension.so
 lib/mini_racer_loader.so
 *.bundle
+.vscode/
diff --git a/README.md b/README.md
index 96abf14..9d6a7a2 100644
--- a/README.md
+++ b/README.md
@@ -85,6 +85,19 @@ context.eval 'var a = new Array(10000); while(true) {a = a.concat(new Array(1000
 # => V8OutOfMemoryError is raised
 ```
 
+### Object marshal max stackdepth support
+
+Contexts can specify a stack depth limit for object marshalling. Max depth is unbounded by default.
+
+```ruby
+# terminates script if stack depth exceeds max during marshal
+context = MiniRacer::Context.new(marshal_stack_depth: 500)
+context.attach("a", proc{|a| a})
+
+context.eval("let arr = []; arr.push(arr); a(arr)")
+# => RuntimeError is raised
+```
+
 ### Rich debugging with "filename" support
 
 ```ruby
diff --git a/ext/mini_racer_extension/mini_racer_extension.cc b/ext/mini_racer_extension/mini_racer_extension.cc
index fc8219c..255a296 100644
--- a/ext/mini_racer_extension/mini_racer_extension.cc
+++ b/ext/mini_racer_extension/mini_racer_extension.cc
@@ -115,6 +115,7 @@ typedef struct {
     useconds_t timeout;
     EvalResult* result;
     size_t max_memory;
+    size_t marshal_stackdepth;
 } EvalParams;
 
 typedef struct {
@@ -126,6 +127,7 @@ typedef struct {
     Local<Value> *argv;
     EvalResult result;
     size_t max_memory;
+    size_t marshal_stackdepth;
 } FunctionCall;
 
 class IsolateData {
@@ -136,16 +138,27 @@ public:
         IN_GVL, // whether we are inside of ruby gvl or not
         DO_TERMINATE, // terminate as soon as possible
         MEM_SOFTLIMIT_REACHED, // we've hit the memory soft limit
-        MEM_SOFTLIMIT_VALUE,
+        MEM_SOFTLIMIT_MAX, // maximum memory value
+        MARSHAL_STACKDEPTH_REACHED, // we've hit our max stack depth
+        MARSHAL_STACKDEPTH_VALUE, // current stackdepth
+        MARSHAL_STACKDEPTH_MAX, // maximum stack depth during marshal
     };
 
+    static void Init(Isolate *isolate) {
+        // zero out all fields in the bitfield
+        isolate->SetData(0, 0);
+    }
+
     static uintptr_t Get(Isolate *isolate, Flag flag) {
         Bitfield u = { reinterpret_cast<uint64_t>(isolate->GetData(0)) };
         switch (flag) {
             case IN_GVL: return u.IN_GVL;
             case DO_TERMINATE: return u.DO_TERMINATE;
             case MEM_SOFTLIMIT_REACHED: return u.MEM_SOFTLIMIT_REACHED;
-            case MEM_SOFTLIMIT_VALUE: return u.MEM_SOFTLIMIT_VALUE << 10;
+            case MEM_SOFTLIMIT_MAX: return u.MEM_SOFTLIMIT_MAX << 10;
+            case MARSHAL_STACKDEPTH_REACHED: return u.MARSHAL_STACKDEPTH_REACHED;
+            case MARSHAL_STACKDEPTH_VALUE: return u.MARSHAL_STACKDEPTH_VALUE;
+            case MARSHAL_STACKDEPTH_MAX: return u.MARSHAL_STACKDEPTH_MAX;
         }
     }
 
@@ -156,7 +169,10 @@ public:
             case DO_TERMINATE: u.DO_TERMINATE = value; break;
             case MEM_SOFTLIMIT_REACHED: u.MEM_SOFTLIMIT_REACHED = value; break;
             // drop least significant 10 bits 'store memory amount in kb'
-            case MEM_SOFTLIMIT_VALUE: u.MEM_SOFTLIMIT_VALUE = value >> 10; break;
+            case MEM_SOFTLIMIT_MAX: u.MEM_SOFTLIMIT_MAX = value >> 10; break;
+            case MARSHAL_STACKDEPTH_REACHED: u.MARSHAL_STACKDEPTH_REACHED = value; break;
+            case MARSHAL_STACKDEPTH_VALUE: u.MARSHAL_STACKDEPTH_VALUE = value; break;
+            case MARSHAL_STACKDEPTH_MAX: u.MARSHAL_STACKDEPTH_MAX = value; break;
         }
         isolate->SetData(0, reinterpret_cast<void*>(u.dataPtr));
     }
@@ -174,15 +190,88 @@ private:
             // order in this struct matters. For cpu performance keep larger subobjects
             //  aligned on their boundaries (8 16 32), try not to straddle
             struct {
-                size_t MEM_SOFTLIMIT_VALUE:22;
+                size_t MEM_SOFTLIMIT_MAX:22;
                 bool IN_GVL:1;
                 bool DO_TERMINATE:1;
                 bool MEM_SOFTLIMIT_REACHED:1;
+                bool MARSHAL_STACKDEPTH_REACHED:1;
+                uint8_t :0; // align to next 8bit bound
+                size_t MARSHAL_STACKDEPTH_VALUE:10;
+                uint8_t :0; // align to next 8bit bound
+                size_t MARSHAL_STACKDEPTH_MAX:10;
             };
         };
     };
 };
 
+struct StackCounter {
+    static void Reset(Isolate* isolate) {
+        if (IsolateData::Get(isolate, IsolateData::MARSHAL_STACKDEPTH_MAX) > 0) {
+            IsolateData::Set(isolate, IsolateData::MARSHAL_STACKDEPTH_VALUE, 0);
+            IsolateData::Set(isolate, IsolateData::MARSHAL_STACKDEPTH_REACHED, false);
+        }
+    }
+
+    static void SetMax(Isolate* isolate, size_t marshalMaxStackDepth) {
+        if (marshalMaxStackDepth > 0) {
+            IsolateData::Set(isolate, IsolateData::MARSHAL_STACKDEPTH_MAX, marshalMaxStackDepth);
+            IsolateData::Set(isolate, IsolateData::MARSHAL_STACKDEPTH_VALUE, 0);
+            IsolateData::Set(isolate, IsolateData::MARSHAL_STACKDEPTH_REACHED, false);
+        }
+    }
+
+    StackCounter(Isolate* isolate) {
+        this->isActive = IsolateData::Get(isolate, IsolateData::MARSHAL_STACKDEPTH_MAX) > 0;
+
+        if (this->isActive) {
+            this->isolate = isolate;
+            this->IncDepth(1);
+        }
+    }
+
+    bool IsTooDeep() {
+        if (!this->IsActive()) {
+            return false;
+        }
+
+        size_t depth = IsolateData::Get(this->isolate, IsolateData::MARSHAL_STACKDEPTH_VALUE);
+        size_t maxDepth = IsolateData::Get(this->isolate, IsolateData::MARSHAL_STACKDEPTH_MAX);
+        if (depth > maxDepth) {
+            IsolateData::Set(this->isolate, IsolateData::MARSHAL_STACKDEPTH_REACHED, true);
+            return true;
+        }
+
+        return false;
+    }
+
+    bool IsActive() {
+        return this->isActive && !IsolateData::Get(this->isolate, IsolateData::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 = IsolateData::Get(this->isolate, IsolateData::MARSHAL_STACKDEPTH_VALUE);
+
+        // don't decrement past 0
+        if (inc > 0 || depth > 0) {
+            depth += inc;
+        }
+
+        IsolateData::Set(this->isolate, IsolateData::MARSHAL_STACKDEPTH_VALUE, depth);
+    }
+};
+
 static VALUE rb_cContext;
 static VALUE rb_cSnapshot;
 static VALUE rb_cIsolate;
@@ -257,7 +346,7 @@ static void gc_callback(Isolate *isolate, GCType type, GCCallbackFlags flags) {
         return;
     }
 
-    size_t softlimit = IsolateData::Get(isolate, IsolateData::MEM_SOFTLIMIT_VALUE);
+    size_t softlimit = IsolateData::Get(isolate, IsolateData::MEM_SOFTLIMIT_MAX);
 
     HeapStatistics stats;
     isolate->GetHeapStatistics(&stats);
@@ -376,10 +465,7 @@ nogvl_context_eval(void* arg) {
     Context::Scope context_scope(context);
     v8::ScriptOrigin *origin = NULL;
 
-    IsolateData::Set(isolate, IsolateData::IN_GVL, false);
-    IsolateData::Set(isolate, IsolateData::DO_TERMINATE, false);
-    IsolateData::Set(isolate, IsolateData::MEM_SOFTLIMIT_VALUE, 0);
-    IsolateData::Set(isolate, IsolateData::MEM_SOFTLIMIT_REACHED, false);
+    IsolateData::Init(isolate);
 
     MaybeLocal<Script> parsed_script;
 
@@ -406,13 +492,17 @@ nogvl_context_eval(void* arg) {
     } else {
         // parsing successful
         if (eval_params->max_memory > 0) {
-            IsolateData::Set(isolate, IsolateData::MEM_SOFTLIMIT_VALUE, eval_params->max_memory);
+            IsolateData::Set(isolate, IsolateData::MEM_SOFTLIMIT_MAX, eval_params->max_memory);
             if (!isolate_info->added_gc_cb) {
             isolate->AddGCEpilogueCallback(gc_callback);
                 isolate_info->added_gc_cb = true;
             }
         }
 
+        if (eval_params->marshal_stackdepth > 0) {
+            StackCounter::SetMax(isolate, eval_params->marshal_stackdepth);
+        }
+
         maybe_value = parsed_script.ToLocalChecked()->Run(context);
     }
 
@@ -436,6 +526,18 @@ static VALUE convert_v8_to_ruby(Isolate* isolate, Local<Context> context,
     Isolate::Scope isolate_scope(isolate);
     HandleScope scope(isolate);
 
+    StackCounter stackCounter(isolate);
+
+    if (IsolateData::Get(isolate, IsolateData::MARSHAL_STACKDEPTH_REACHED)) {
+        return Qnil;
+    }
+
+    if (stackCounter.IsTooDeep()) {
+        IsolateData::Set(isolate, IsolateData::DO_TERMINATE, true);
+        isolate->TerminateExecution();
+        return Qnil;
+    }
+
     if (value->IsNull() || value->IsUndefined()){
         return Qnil;
     }
@@ -931,8 +1033,13 @@ 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 = IsolateData::Get(isolate, IsolateData::MEM_SOFTLIMIT_REACHED);
+            bool marshal_stack_maxdepth_reached = IsolateData::Get(isolate, IsolateData::MARSHAL_STACKDEPTH_REACHED);
             // 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;
@@ -967,6 +1074,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);
         }
 
@@ -1024,6 +1132,7 @@ static VALUE rb_context_eval_unsafe(VALUE self, VALUE str, VALUE filename) {
         eval_params.result = &eval_result;
         eval_params.timeout = 0;
         eval_params.max_memory = 0;
+        eval_params.marshal_stackdepth = 0;
         VALUE timeout = rb_iv_get(self, "@timeout");
         if (timeout != Qnil) {
             eval_params.timeout = (useconds_t)NUM2LONG(timeout);
@@ -1034,6 +1143,11 @@ static VALUE rb_context_eval_unsafe(VALUE self, VALUE str, VALUE filename) {
             eval_params.max_memory = (size_t)NUM2ULONG(mem_softlimit);
         }
 
+        VALUE stack_depth = rb_iv_get(self, "@marshal_stack_depth");
+        if (stack_depth != Qnil) {
+            eval_params.marshal_stackdepth = (size_t)NUM2ULONG(stack_depth);
+        }
+
         eval_result.message = NULL;
         eval_result.backtrace = NULL;
 
@@ -1103,6 +1217,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);
@@ -1556,12 +1671,16 @@ nogvl_context_call(void *args) {
     IsolateData::Set(isolate, IsolateData::DO_TERMINATE, false);
 
     if (call->max_memory > 0) {
-        IsolateData::Set(isolate, IsolateData::MEM_SOFTLIMIT_VALUE, call->max_memory);
+        IsolateData::Set(isolate, IsolateData::MEM_SOFTLIMIT_MAX, call->max_memory);
         IsolateData::Set(isolate, IsolateData::MEM_SOFTLIMIT_REACHED, false);
         if (!isolate_info->added_gc_cb) {
-        isolate->AddGCEpilogueCallback(gc_callback);
+            isolate->AddGCEpilogueCallback(gc_callback);
             isolate_info->added_gc_cb = true;
+        }
     }
+ 
+    if (call->marshal_stackdepth > 0) {
+        StackCounter::SetMax(isolate, call->marshal_stackdepth);
     }
 
     Isolate::Scope isolate_scope(isolate);
@@ -1627,6 +1746,13 @@ static VALUE rb_context_call_unsafe(int argc, VALUE *argv, VALUE self) {
         unsigned long sl_int = NUM2ULONG(mem_softlimit);
         call.max_memory = (size_t)sl_int;
     }
+ 
+    call.marshal_stackdepth = 0;
+    VALUE marshal_stackdepth = rb_iv_get(self, "@marshal_stack_depth");
+    if (marshal_stackdepth != Qnil) {
+        unsigned long sl_int = NUM2ULONG(marshal_stackdepth);
+        call.marshal_stackdepth = (size_t)sl_int;
+    }
 
     bool missingFunction = false;
     {
diff --git a/lib/mini_racer.rb b/lib/mini_racer.rb
index 98bd1c7..7f57d6b 100644
--- a/lib/mini_racer.rb
+++ b/lib/mini_racer.rb
@@ -15,6 +15,9 @@ require "json"
 
 module MiniRacer
 
+  MARSHAL_STACKDEPTH_DEFAULT = 2**9-2
+  MARSHAL_STACKDEPTH_MAX_VALUE = 2**10-2
+
   class Error < ::StandardError; end
 
   class ContextDisposedError < Error; end
@@ -140,10 +143,10 @@ module MiniRacer
       end
     end
 
-    def initialize(max_memory: nil, timeout: nil, isolate: nil, ensure_gc_after_idle: nil, snapshot: nil)
+    def initialize(max_memory: nil, timeout: nil, isolate: nil, ensure_gc_after_idle: nil, snapshot: nil, marshal_stack_depth: nil)
       options ||= {}
 
-      check_init_options!(isolate: isolate, snapshot: snapshot, max_memory: max_memory, ensure_gc_after_idle: ensure_gc_after_idle, timeout: timeout)
+      check_init_options!(isolate: isolate, snapshot: snapshot, max_memory: max_memory, marshal_stack_depth: marshal_stack_depth, ensure_gc_after_idle: ensure_gc_after_idle, timeout: timeout)
 
       @functions = {}
       @timeout = nil
@@ -151,6 +154,7 @@ module MiniRacer
       @current_exception = nil
       @timeout = timeout
       @max_memory = max_memory
+      @marshal_stack_depth = marshal_stack_depth
 
       # false signals it should be fetched if requested
       @isolate = isolate || false
@@ -379,11 +383,12 @@ module MiniRacer
       rp.close if rp
     end
 
-    def check_init_options!(isolate:, snapshot:, max_memory:, ensure_gc_after_idle:, timeout:)
+    def check_init_options!(isolate:, snapshot:, max_memory:, marshal_stack_depth:, ensure_gc_after_idle:, timeout:)
       assert_option_is_nil_or_a('isolate', isolate, Isolate)
       assert_option_is_nil_or_a('snapshot', snapshot, Snapshot)
 
-      assert_numeric_or_nil('max_memory', max_memory, min_value: 10_000, max_value: 2**32)
+      assert_numeric_or_nil('max_memory', max_memory, min_value: 10_000, max_value: 2**32-1)
+      assert_numeric_or_nil('marshal_stack_depth', marshal_stack_depth, min_value: 1, max_value: MARSHAL_STACKDEPTH_MAX_VALUE)
       assert_numeric_or_nil('ensure_gc_after_idle', ensure_gc_after_idle, min_value: 1)
       assert_numeric_or_nil('timeout', timeout, min_value: 1)
 
diff --git a/test/mini_racer_test.rb b/test/mini_racer_test.rb
index 409f21f..b882ebb 100644
--- a/test/mini_racer_test.rb
+++ b/test/mini_racer_test.rb
@@ -340,7 +340,7 @@ raise FooError, "I like foos"
     end
 
     assert_raises(ArgumentError) do
-      MiniRacer::Context.new(max_memory: 2**32+1)
+      MiniRacer::Context.new(max_memory: 2**32)
     end
   end
 
@@ -868,6 +868,65 @@ raise FooError, "I like foos"
     assert_equal :foo, context.eval("Symbol('foo')")
   end
 
+  def test_cyclical_object_js
+    context = MiniRacer::Context.new(marshal_stack_depth: 5)
+    context.attach("a", proc{|a| a})
+
+    assert_raises(MiniRacer::RuntimeError) { context.eval("var o={}; o.o=o; a(o)") }
+  end
+
+  def test_cyclical_array_js
+    context = MiniRacer::Context.new(marshal_stack_depth: 5)
+    context.attach("a", proc{|a| a})
+
+    assert_raises(MiniRacer::RuntimeError) { context.eval("let arr = []; arr.push(arr); a(arr)") }
+  end
+
+  def test_cyclical_elem_in_array_js
+    context = MiniRacer::Context.new(marshal_stack_depth: 5)
+    context.attach("a", proc{|a| a})
+
+    assert_raises(MiniRacer::RuntimeError) { context.eval("let arr = []; arr[0]=1; arr[1]=arr; a(arr)") }
+  end
+
+  def test_infinite_object_js
+    context = MiniRacer::Context.new(marshal_stack_depth: 5)
+    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(marshal_stack_depth: 5)
+    context.attach("a", proc{|a| a})
+
+    # stack depth should be enough to marshal the object
+    assert_equal [[[]]], context.eval("let arr = [[[]]]; a(arr)")
+
+    # too deep
+    assert_raises(MiniRacer::RuntimeError) { context.eval("let arr = [[[[[[[[]]]]]]]]; a(arr)") }
+  end
+
+  def test_stackdepth_bounds
+    assert_raises(ArgumentError) do
+      MiniRacer::Context.new(marshal_stack_depth: -2)
+    end
+
+    assert_raises(ArgumentError) do
+      MiniRacer::Context.new(marshal_stack_depth: MiniRacer::MARSHAL_STACKDEPTH_MAX_VALUE+1)
+    end
+  end
+
   def test_proxy_support
     js = <<~JS
       function MyProxy(reference) {