From d9dfffa4c807a755a2a246b14708b07cabf50e55 Mon Sep 17 00:00:00 2001
From: Luca Casonato <hello@lcas.dev>
Date: Sat, 29 Apr 2023 17:43:07 +0200
Subject: [PATCH] fix(ext/kv): stricter structured clone serializer (#18914)

---
 cli/tests/unit/kv_test.ts | 30 ++++++++++++++++++++++++++++++
 core/ops_builtin_v8.rs    | 21 ++++++++++++++++++++-
 ext/kv/01_db.ts           |  4 ++--
 3 files changed, 52 insertions(+), 3 deletions(-)

diff --git a/cli/tests/unit/kv_test.ts b/cli/tests/unit/kv_test.ts
index 0dc1690aad..5a202fb0be 100644
--- a/cli/tests/unit/kv_test.ts
+++ b/cli/tests/unit/kv_test.ts
@@ -123,6 +123,36 @@ dbTest("set and get recursive object", async (db) => {
   assert(resultValue.a === resultValue);
 });
 
+// invalid values (as per structured clone algorithm with _for storage_, NOT JSON)
+const INVALID_VALUE_CASES = [
+  { name: "function", value: () => {} },
+  { name: "symbol", value: Symbol() },
+  { name: "WeakMap", value: new WeakMap() },
+  { name: "WeakSet", value: new WeakSet() },
+  {
+    name: "WebAssembly.Module",
+    value: new WebAssembly.Module(
+      new Uint8Array([0x00, 0x61, 0x73, 0x6D, 0x01, 0x00, 0x00, 0x00]),
+    ),
+  },
+  {
+    name: "SharedArrayBuffer",
+    value: new SharedArrayBuffer(3),
+  },
+];
+
+for (const { name, value } of INVALID_VALUE_CASES) {
+  dbTest(`set and get ${name} value (invalid)`, async (db) => {
+    await assertRejects(
+      async () => await db.set(["a"], value),
+      Error,
+    );
+    const res = await db.get(["a"]);
+    assertEquals(res.key, ["a"]);
+    assertEquals(res.value, null);
+  });
+}
+
 const keys = [
   ["a"],
   ["a", "b"],
diff --git a/core/ops_builtin_v8.rs b/core/ops_builtin_v8.rs
index 67cf1222f1..bc4f906e27 100644
--- a/core/ops_builtin_v8.rs
+++ b/core/ops_builtin_v8.rs
@@ -211,6 +211,7 @@ fn op_decode<'a>(
 struct SerializeDeserialize<'a> {
   host_objects: Option<v8::Local<'a, v8::Array>>,
   error_callback: Option<v8::Local<'a, v8::Function>>,
+  for_storage: bool,
 }
 
 impl<'a> v8::ValueSerializerImpl for SerializeDeserialize<'a> {
@@ -238,6 +239,9 @@ impl<'a> v8::ValueSerializerImpl for SerializeDeserialize<'a> {
     scope: &mut v8::HandleScope<'s>,
     shared_array_buffer: v8::Local<'s, v8::SharedArrayBuffer>,
   ) -> Option<u32> {
+    if self.for_storage {
+      return None;
+    }
     let state_rc = JsRuntime::state(scope);
     let state = state_rc.borrow_mut();
     if let Some(shared_array_buffer_store) = &state.shared_array_buffer_store {
@@ -254,6 +258,11 @@ impl<'a> v8::ValueSerializerImpl for SerializeDeserialize<'a> {
     scope: &mut v8::HandleScope<'_>,
     module: v8::Local<v8::WasmModuleObject>,
   ) -> Option<u32> {
+    if self.for_storage {
+      let message = v8::String::new(scope, "Wasm modules cannot be stored")?;
+      self.throw_data_clone_error(scope, message);
+      return None;
+    }
     let state_rc = JsRuntime::state(scope);
     let state = state_rc.borrow_mut();
     if let Some(compiled_wasm_module_store) = &state.compiled_wasm_module_store
@@ -293,6 +302,9 @@ impl<'a> v8::ValueDeserializerImpl for SerializeDeserialize<'a> {
     scope: &mut v8::HandleScope<'s>,
     transfer_id: u32,
   ) -> Option<v8::Local<'s, v8::SharedArrayBuffer>> {
+    if self.for_storage {
+      return None;
+    }
     let state_rc = JsRuntime::state(scope);
     let state = state_rc.borrow_mut();
     if let Some(shared_array_buffer_store) = &state.shared_array_buffer_store {
@@ -310,6 +322,9 @@ impl<'a> v8::ValueDeserializerImpl for SerializeDeserialize<'a> {
     scope: &mut v8::HandleScope<'s>,
     clone_id: u32,
   ) -> Option<v8::Local<'s, v8::WasmModuleObject>> {
+    if self.for_storage {
+      return None;
+    }
     let state_rc = JsRuntime::state(scope);
     let state = state_rc.borrow_mut();
     if let Some(compiled_wasm_module_store) = &state.compiled_wasm_module_store
@@ -337,7 +352,7 @@ impl<'a> v8::ValueDeserializerImpl for SerializeDeserialize<'a> {
       }
     }
 
-    let message =
+    let message: v8::Local<v8::String> =
       v8::String::new(scope, "Failed to deserialize host object").unwrap();
     let error = v8::Exception::error(scope, message);
     scope.throw_exception(error);
@@ -350,6 +365,8 @@ impl<'a> v8::ValueDeserializerImpl for SerializeDeserialize<'a> {
 struct SerializeDeserializeOptions<'a> {
   host_objects: Option<serde_v8::Value<'a>>,
   transferred_array_buffers: Option<serde_v8::Value<'a>>,
+  #[serde(default)]
+  for_storage: bool,
 }
 
 #[op(v8)]
@@ -385,6 +402,7 @@ fn op_serialize(
   let serialize_deserialize = Box::new(SerializeDeserialize {
     host_objects,
     error_callback,
+    for_storage: options.for_storage,
   });
   let mut value_serializer =
     v8::ValueSerializer::new(scope, serialize_deserialize);
@@ -464,6 +482,7 @@ fn op_deserialize<'a>(
   let serialize_deserialize = Box::new(SerializeDeserialize {
     host_objects,
     error_callback: None,
+    for_storage: options.for_storage,
   });
   let mut value_deserializer =
     v8::ValueDeserializer::new(scope, serialize_deserialize, &zero_copy);
diff --git a/ext/kv/01_db.ts b/ext/kv/01_db.ts
index 0dd6ba83a2..72d3580051 100644
--- a/ext/kv/01_db.ts
+++ b/ext/kv/01_db.ts
@@ -312,7 +312,7 @@ function deserializeValue(entry: RawKvEntry): Deno.KvEntry<unknown> {
     case "v8":
       return {
         ...entry,
-        value: core.deserialize(value),
+        value: core.deserialize(value, { forStorage: true }),
       };
     case "bytes":
       return {
@@ -343,7 +343,7 @@ function serializeValue(value: unknown): RawValue {
   } else {
     return {
       kind: "v8",
-      value: core.serialize(value),
+      value: core.serialize(value, { forStorage: true }),
     };
   }
 }