From 770d1f17158488fa072aa23991d3cf1b78d9c97e Mon Sep 17 00:00:00 2001
From: Aapo Alasuutari <aapo.alasuutari@gmail.com>
Date: Fri, 5 Aug 2022 19:26:54 +0300
Subject: [PATCH] fix(ext/ffi): Check CStr for UTF-8 validity on read (#15318)

Co-authored-by: Phosra <phosra@tutanota.com>
---
 ext/ffi/lib.rs         | 15 ++++++++-------
 test_ffi/src/lib.rs    |  7 +++++++
 test_ffi/tests/test.js | 24 ++++++++++++++++++++++++
 3 files changed, 39 insertions(+), 7 deletions(-)

diff --git a/ext/ffi/lib.rs b/ext/ffi/lib.rs
index ec80d47864..ae4ddd6057 100644
--- a/ext/ffi/lib.rs
+++ b/ext/ffi/lib.rs
@@ -2187,13 +2187,14 @@ where
   permissions.check(None)?;
 
   // SAFETY: Pointer is user provided.
-  let cstr = unsafe { CStr::from_ptr(ptr as *const c_char) }.to_bytes();
-  let value: v8::Local<v8::Value> =
-    v8::String::new_from_utf8(scope, cstr, v8::NewStringType::Normal)
-      .ok_or_else(|| {
-        type_error("Invalid CString pointer, string exceeds max length")
-      })?
-      .into();
+  let cstr = unsafe { CStr::from_ptr(ptr as *const c_char) }
+    .to_str()
+    .map_err(|_| type_error("Invalid CString pointer, not valid UTF-8"))?;
+  let value: v8::Local<v8::Value> = v8::String::new(scope, cstr)
+    .ok_or_else(|| {
+      type_error("Invalid CString pointer, string exceeds max length")
+    })?
+    .into();
   Ok(value.into())
 }
 
diff --git a/test_ffi/src/lib.rs b/test_ffi/src/lib.rs
index b061ca8d5b..08ef7cb7f1 100644
--- a/test_ffi/src/lib.rs
+++ b/test_ffi/src/lib.rs
@@ -411,3 +411,10 @@ static STRING: &str = "Hello, world!\0";
 extern "C" fn ffi_string() -> *const u8 {
   STRING.as_ptr()
 }
+
+/// Invalid UTF-8 characters, array of length 14
+#[no_mangle]
+pub static static_char: [u8; 14] = [
+  0xC0, 0xC1, 0xF5, 0xF6, 0xF7, 0xF8, 0xF9, 0xFA, 0xFB, 0xFC, 0xFD, 0xFE, 0xFF,
+  0x00,
+];
diff --git a/test_ffi/tests/test.js b/test_ffi/tests/test.js
index d658ec1697..23b46902a8 100644
--- a/test_ffi/tests/test.js
+++ b/test_ffi/tests/test.js
@@ -3,6 +3,7 @@
 
 // Run using cargo test or `--v8-options=--allow-natives-syntax`
 
+import { assertEquals } from "https://deno.land/std@0.149.0/testing/asserts.ts";
 import {
   assertThrows,
 } from "../../test_util/std/testing/asserts.ts";
@@ -186,6 +187,12 @@ const dylib = Deno.dlopen(libPath, {
   "static_ptr": {
     type: "pointer",
   },
+  /**
+   * Invalid UTF-8 characters, buffer of length 14
+   */
+  "static_char": {
+    type: "pointer",
+  },
 });
 const { symbols } = dylib;
 
@@ -478,6 +485,23 @@ uint32Array[0] = 55; // MUTATES!
 console.log("uint32Array[0] after mutation:", uint32Array[0]);
 console.log("Static ptr value after mutation:", view.getUint32());
 
+// Test non-UTF-8 characters
+
+const charView = new Deno.UnsafePointerView(dylib.symbols.static_char);
+
+const charArrayBuffer = charView.getArrayBuffer(14);
+const uint8Array = new Uint8Array(charArrayBuffer);
+assertEquals([...uint8Array], [
+  0xC0, 0xC1, 0xF5, 0xF6, 0xF7, 0xF8, 0xF9, 0xFA, 0xFB, 0xFC, 0xFD, 0xFE, 0xFF,
+  0x00
+]);
+
+try {
+  assertThrows(() => charView.getCString(), TypeError, "Invalid CString pointer, not valid UTF-8");
+} catch (_err) {
+  console.log("Invalid UTF-8 characters to `v8::String`:", charView.getCString());
+}
+
 (function cleanup() {
   dylib.close();
   throwCallback.close();