Only enable JS runtime limits during execution (microsoft#5730)

.daily_canary

@@ -1,4 +1,4 @@
    -^-     ___           ___
   (- -)   (= =)   |     Y   &  +--?
  (  V  ) /  .  \  |    +---=---'
-/--x-m- /--n-n---xXx--/--yY------>>>----<<<
+/--x-m- /--n-n---xXx--/--yY------>>>----<<<>>

CHANGELOG.md

@@ -5,6 +5,13 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
+## [5.0.0-dev5]
+
+[5.0.0-dev5]: https://github.com/microsoft/CCF/releases/tag/ccf-5.0.0-dev5
+
+- In governance contexts, JS runtimes now only use runtime limits from the [public:ccf.gov.js_runtime_options map](https://microsoft.github.io/CCF/main/audit/builtin_maps.html#js-runtime-options) if they are strictly higher than the defaults (#5730).
+- Fixed an issue where a JS runtime limit could be hit out of user code execution, leading to an incorrectly constructed JS runtime or a crash (#5730).
+
 ## [5.0.0-dev4]
 
 [5.0.0-dev4]: https://github.com/microsoft/CCF/releases/tag/ccf-5.0.0-dev4

src/apps/js_generic/js_generic_base.cpp

@@ -312,8 +312,9 @@ namespace ccfapp
       // Update the top of the stack for the current thread, used by the stack
       // guard Note this is only active outside SGX
       JS_UpdateStackTop(ctx.runtime());
+      // Make the heap and stack limits safe while we init the runtime
+      ctx.runtime().reset_runtime_options();
 
-      ctx.runtime().set_runtime_options(&endpoint_ctx.tx);
       JS_SetModuleLoaderFunc(
         ctx.runtime(), nullptr, js::js_app_module_loader, &endpoint_ctx.tx);
 
@@ -353,7 +354,12 @@ namespace ccfapp
 
       // Call exported function
       auto request = create_request_obj(endpoint, endpoint_ctx, ctx);
-      auto val = ctx.call(export_func, {request});
+
+      auto val = ctx.call_with_rt_options(
+        export_func,
+        {request},
+        &endpoint_ctx.tx,
+        ccf::js::RuntimeLimitsPolicy::NONE);
 
       if (JS_IsException(val))
       {

src/js/wrap.cpp

@@ -21,6 +21,7 @@
 #include "node/rpc/jwt_management.h"
 #include "node/rpc/node_interface.h"
 
+#include <algorithm>
 #include <memory>
 #include <quickjs/quickjs-exports.h>
 #include <quickjs/quickjs.h>
@@ -137,7 +138,7 @@ namespace ccf::js
     }
   }
 
-  JSWrappedValue Context::call(
+  JSWrappedValue Context::inner_call(
     const JSWrappedValue& f, const std::vector<js::JSWrappedValue>& argv)
   {
     std::vector<JSValue> argvn;
@@ -146,14 +147,28 @@ namespace ccf::js
     {
       argvn.push_back(a.val);
     }
+
+    return W(JS_Call(
+      ctx, f, ccf::js::constants::Undefined, argv.size(), argvn.data()));
+  }
+
+  JSWrappedValue Context::call_with_rt_options(
+    const JSWrappedValue& f,
+    const std::vector<js::JSWrappedValue>& argv,
+    kv::Tx* tx,
+    RuntimeLimitsPolicy policy)
+  {
+    rt.set_runtime_options(tx, policy);
     const auto curr_time = ccf::get_enclave_time();
     interrupt_data.start_time = curr_time;
     interrupt_data.max_execution_time = rt.get_max_exec_time();
-    interrupt_data.access = access;
     JS_SetInterruptHandler(rt, js_custom_interrupt_handler, &interrupt_data);
 
-    return W(JS_Call(
-      ctx, f, ccf::js::constants::Undefined, argv.size(), argvn.data()));
+    auto rv = inner_call(f, argv);
+
+    rt.reset_runtime_options();
+
+    return rv;
   }
 
   Runtime::Runtime()
@@ -388,7 +403,7 @@ namespace ccf::js
           jsctx.new_array_buffer_copy(k.data(), k.size()),
           obj};
 
-        auto val = jsctx.call(func, args);
+        auto val = jsctx.inner_call(func, args);
 
         if (JS_IsException(val))
         {
@@ -1483,7 +1498,8 @@ namespace ccf::js
     auto& tx = *tx_ctx_ptr->tx;
 
     js::Context ctx2(js::TxAccess::APP);
-    ctx2.runtime().set_runtime_options(tx_ctx_ptr->tx);
+    ctx2.runtime().set_runtime_options(
+      tx_ctx_ptr->tx, js::RuntimeLimitsPolicy::NO_LOWER_THAN_DEFAULTS);
     JS_SetModuleLoaderFunc(
       ctx2.runtime(), nullptr, js::js_app_module_loader, &tx);
 
@@ -2355,7 +2371,14 @@ namespace ccf::js
     }
   }
 
-  void Runtime::set_runtime_options(kv::Tx* tx)
+  void Runtime::reset_runtime_options()
+  {
+    JS_SetMaxStackSize(rt, 0);
+    JS_SetMemoryLimit(rt, -1);
+    JS_SetInterruptHandler(rt, NULL, NULL);
+  }
+
+  void Runtime::set_runtime_options(kv::Tx* tx, RuntimeLimitsPolicy policy)
   {
     size_t stack_size = default_stack_size;
     size_t heap_size = default_heap_size;
@@ -2365,10 +2388,20 @@ namespace ccf::js
 
     if (js_runtime_options.has_value())
     {
-      heap_size = js_runtime_options.value().max_heap_bytes;
-      stack_size = js_runtime_options.value().max_stack_bytes;
-      max_exec_time = std::chrono::milliseconds{
-        js_runtime_options.value().max_execution_time_ms};
+      bool no_lower_than_defaults =
+        policy == RuntimeLimitsPolicy::NO_LOWER_THAN_DEFAULTS;
+
+      heap_size = std::max(
+        js_runtime_options.value().max_heap_bytes,
+        no_lower_than_defaults ? default_heap_size : 0);
+      stack_size = std::max(
+        js_runtime_options.value().max_stack_bytes,
+        no_lower_than_defaults ? default_stack_size : 0);
+      max_exec_time = std::max(
+        std::chrono::milliseconds{
+          js_runtime_options.value().max_execution_time_ms},
+        no_lower_than_defaults ? default_max_execution_time :
+                                 std::chrono::milliseconds{0});
       log_exception_details = js_runtime_options.value().log_exception_details;
       return_exception_details =
         js_runtime_options.value().return_exception_details;

src/js/wrap.h

@@ -43,6 +43,12 @@ namespace ccf::js
   const size_t default_stack_size = 1024 * 1024;
   const size_t default_heap_size = 100 * 1024 * 1024;
 
+  enum class RuntimeLimitsPolicy
+  {
+    NONE,
+    NO_LOWER_THAN_DEFAULTS
+  };
+
   /// Describes the context in which JS script is currently executing. Used to
   /// determine which KV tables should be accessible.
   enum class TxAccess
@@ -224,6 +230,8 @@ namespace ccf::js
   void js_dump_error(JSContext* ctx);
   std::pair<std::string, std::optional<std::string>> js_error_message(
     Context& ctx);
+  std::pair<std::string, std::optional<std::string>> js_error_message_from_val(
+    Context& ctx, JSWrappedValue& exc);
 
   JSValue js_body_text(
     JSContext* ctx,
@@ -276,7 +284,8 @@ namespace ccf::js
       return rt;
     }
 
-    void set_runtime_options(kv::Tx* tx);
+    void reset_runtime_options();
+    void set_runtime_options(kv::Tx* tx, RuntimeLimitsPolicy policy);
 
     std::chrono::milliseconds get_max_exec_time() const
     {
@@ -517,7 +526,17 @@ namespace ccf::js
       return W(JS_GetException(ctx));
     }
 
-    JSWrappedValue call(
+    JSWrappedValue call_with_rt_options(
+      const JSWrappedValue& f,
+      const std::vector<js::JSWrappedValue>& argv,
+      kv::Tx* tx,
+      RuntimeLimitsPolicy policy);
+
+    // Call a JS function _without_ any stack, heap or execution time limits.
+    // Only to be used, as the name indicates, for calls inside an already
+    // invoked JS function, where the caller has already set up the necessary
+    // limits.
+    JSWrappedValue inner_call(
       const JSWrappedValue& f, const std::vector<js::JSWrappedValue>& argv);
 
     JSWrappedValue parse_json(const nlohmann::json& j) const

src/node/gov/handlers/proposals.h

@@ -140,7 +140,6 @@ namespace ccf::gov::endpoints
       for (const auto& [mid, mb] : proposal_info.ballots)
       {
         js::Context js_context(js::TxAccess::GOV_RO);
-        js_context.runtime().set_runtime_options(&tx);
         js::TxContext txctx{&tx};
         js::populate_global_ccf_kv(&txctx, js_context);
         auto ballot_func = js_context.function(
@@ -156,7 +155,12 @@ namespace ccf::gov::endpoints
             proposal_info.proposer_id.data(),
             proposal_info.proposer_id.size())};
 
-        auto val = js_context.call(ballot_func, argv);
+        auto val = js_context.call_with_rt_options(
+          ballot_func,
+          argv,
+          &tx,
+          js::RuntimeLimitsPolicy::NO_LOWER_THAN_DEFAULTS);
+
         if (!JS_IsException(val))
         {
           votes.emplace_back(mid, JS_ToBool(js_context, val));
@@ -182,7 +186,6 @@ namespace ccf::gov::endpoints
       {
         {
           js::Context js_context(js::TxAccess::GOV_RO);
-          js_context.runtime().set_runtime_options(&tx);
           js::TxContext txctx{&tx};
           js::populate_global_ccf_kv(&txctx, js_context);
           auto resolve_func = js_context.function(
@@ -213,7 +216,11 @@ namespace ccf::gov::endpoints
           }
           argv.push_back(vs);
 
-          auto val = js_context.call(resolve_func, argv);
+          auto val = js_context.call_with_rt_options(
+            resolve_func,
+            argv,
+            &tx,
+            js::RuntimeLimitsPolicy::NO_LOWER_THAN_DEFAULTS);
 
           if (JS_IsException(val))
           {
@@ -267,7 +274,6 @@ namespace ccf::gov::endpoints
           {
             // Evaluate apply function
             js::Context js_context(js::TxAccess::GOV_RW);
-            js_context.runtime().set_runtime_options(&tx);
             js::TxContext txctx{&tx};
 
             auto gov_effects =
@@ -292,7 +298,11 @@ namespace ccf::gov::endpoints
               js_context.new_string_len(
                 proposal_id.c_str(), proposal_id.size())};
 
-            auto val = js_context.call(apply_func, argv);
+            auto val = js_context.call_with_rt_options(
+              apply_func,
+              argv,
+              &tx,
+              js::RuntimeLimitsPolicy::NO_LOWER_THAN_DEFAULTS);
 
             if (JS_IsException(val))
             {
@@ -438,7 +448,6 @@ namespace ccf::gov::endpoints
             }
 
             js::Context context(js::TxAccess::GOV_RO);
-            context.runtime().set_runtime_options(&ctx.tx);
             js::TxContext txctx{&ctx.tx};
             js::populate_global_ccf_kv(&txctx, context);
 
@@ -450,7 +459,11 @@ namespace ccf::gov::endpoints
             proposal_body = cose_ident.content;
             auto proposal_arg = context.new_string_len(
               (const char*)proposal_body.data(), proposal_body.size());
-            auto validate_result = context.call(validate_func, {proposal_arg});
+            auto validate_result = context.call_with_rt_options(
+              validate_func,
+              {proposal_arg},
+              &ctx.tx,
+              js::RuntimeLimitsPolicy::NO_LOWER_THAN_DEFAULTS);
 
             // Handle error cases of validation
             {

src/node/rpc/member_frontend.h

@@ -149,7 +149,6 @@ namespace ccf
       for (const auto& [mid, mb] : pi_->ballots)
       {
         js::Context context(js::TxAccess::GOV_RO);
-        context.runtime().set_runtime_options(&tx);
         js::TxContext txctx{&tx};
         js::populate_global_ccf_kv(&txctx, context);
         auto ballot_func = context.function(
@@ -163,7 +162,12 @@ namespace ccf
           context.new_string_len(
             pi_->proposer_id.data(), pi_->proposer_id.size())};
 
-        auto val = context.call(ballot_func, argv);
+        auto val = context.call_with_rt_options(
+          ballot_func,
+          argv,
+          &tx,
+          js::RuntimeLimitsPolicy::NO_LOWER_THAN_DEFAULTS);
+
         if (!JS_IsException(val))
         {
           votes.emplace_back(mid, JS_ToBool(context, val));
@@ -187,7 +191,6 @@ namespace ccf
 
       {
         js::Context js_context(js::TxAccess::GOV_RO);
-        js_context.runtime().set_runtime_options(&tx);
         js::TxContext txctx{&tx};
         js::populate_global_ccf_kv(&txctx, js_context);
         auto resolve_func = js_context.function(
@@ -216,7 +219,11 @@ namespace ccf
         }
         argv.push_back(vs);
 
-        auto val = js_context.call(resolve_func, argv);
+        auto val = js_context.call_with_rt_options(
+          resolve_func,
+          argv,
+          &tx,
+          js::RuntimeLimitsPolicy::NO_LOWER_THAN_DEFAULTS);
 
         std::optional<jsgov::Failure> failure = std::nullopt;
         if (JS_IsException(val))
@@ -284,7 +291,6 @@ namespace ccf
           if (pi_.value().state == ProposalState::ACCEPTED)
           {
             js::Context apply_js_context(js::TxAccess::GOV_RW);
-            apply_js_context.runtime().set_runtime_options(&tx);
 
             js::TxContext apply_txctx{&tx};
 
@@ -310,7 +316,11 @@ namespace ccf
               apply_js_context.new_string_len(
                 proposal_id.c_str(), proposal_id.size())};
 
-            auto apply_val = apply_js_context.call(apply_func, apply_argv);
+            auto apply_val = apply_js_context.call_with_rt_options(
+              apply_func,
+              apply_argv,
+              &tx,
+              js::RuntimeLimitsPolicy::NO_LOWER_THAN_DEFAULTS);
 
             if (JS_IsException(apply_val))
             {
@@ -1151,7 +1161,6 @@ namespace ccf
         auto validate_script = constitution.value();
 
         js::Context context(js::TxAccess::GOV_RO);
-        context.runtime().set_runtime_options(&ctx.tx);
         js::TxContext txctx{&ctx.tx};
         js::populate_global_ccf_kv(&txctx, context);
 
@@ -1166,7 +1175,11 @@ namespace ccf
         auto body_len = proposal_body.size();
 
         auto proposal = context.new_string_len(body, body_len);
-        auto val = context.call(validate_func, {proposal});
+        auto val = context.call_with_rt_options(
+          validate_func,
+          {proposal},
+          &ctx.tx,
+          js::RuntimeLimitsPolicy::NO_LOWER_THAN_DEFAULTS);
 
         if (JS_IsException(val))
         {
@@ -1677,7 +1690,8 @@ namespace ccf
 
         {
           js::Context context(js::TxAccess::GOV_RO);
-          context.runtime().set_runtime_options(&ctx.tx);
+          context.runtime().set_runtime_options(
+            &ctx.tx, js::RuntimeLimitsPolicy::NO_LOWER_THAN_DEFAULTS);
           auto ballot_func =
             context.function(params["ballot"], "vote", "body[\"ballot\"]");
         }