From 8e992dc906f8cd5ef38479c22c64d4967d8399cf Mon Sep 17 00:00:00 2001
From: daxpedda <daxpedda@gmail.com>
Date: Mon, 4 Mar 2024 08:51:38 +0100
Subject: [PATCH] Don't allow invalid Unicode scalar values in `char` (#3866)

---
 CHANGELOG.md                         |  3 +++
 crates/cli-support/src/js/binding.rs | 22 +++++++++++++++++++---
 crates/cli-support/src/js/mod.rs     | 13 +++++++++++++
 src/convert/impls.rs                 |  1 +
 tests/wasm/char.js                   | 10 ++++++++++
 tests/wasm/char.rs                   |  5 +++++
 6 files changed, 51 insertions(+), 3 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8004077535b..324e8faaa4f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -26,6 +26,9 @@
 * Make .wasm output deterministic when using `--reference-types`.
   [#3851](https://github.com/rustwasm/wasm-bindgen/pull/3851)
 
+* Don't allow invalid Unicode scalar values in `char`.
+  [#3866](https://github.com/rustwasm/wasm-bindgen/pull/3866)
+
 --------------------------------------------------------------------------------
 
 ## [0.2.91](https://github.com/rustwasm/wasm-bindgen/compare/0.2.90...0.2.91)
diff --git a/crates/cli-support/src/js/binding.rs b/crates/cli-support/src/js/binding.rs
index 4c27d7584e4..ebe4e966c43 100644
--- a/crates/cli-support/src/js/binding.rs
+++ b/crates/cli-support/src/js/binding.rs
@@ -502,6 +502,11 @@ impl<'a, 'b> JsBuilder<'a, 'b> {
         self.prelude(&format!("_assertNonNull({});", arg));
     }
 
+    fn assert_char(&mut self, arg: &str) {
+        self.cx.expose_assert_char();
+        self.prelude(&format!("_assertChar({});", arg));
+    }
+
     fn assert_optional_bigint(&mut self, arg: &str) {
         if !self.cx.config.debug {
             return;
@@ -739,7 +744,11 @@ fn instruction(
 
         Instruction::I32FromStringFirstChar => {
             let val = js.pop();
-            js.push(format!("{}.codePointAt(0)", val));
+            let i = js.tmp();
+            js.prelude(&format!("const char{i} = {val}.codePointAt(0);"));
+            let val = format!("char{i}");
+            js.assert_char(&val);
+            js.push(val);
         }
 
         Instruction::I32FromExternrefOwned => {
@@ -816,11 +825,18 @@ fn instruction(
 
         Instruction::I32FromOptionChar => {
             let val = js.pop();
+            let i = js.tmp();
             js.cx.expose_is_like_none();
-            js.push(format!(
-                "isLikeNone({0}) ? 0xFFFFFF : {0}.codePointAt(0)",
+            js.prelude(&format!(
+                "const char{i} = isLikeNone({0}) ? 0xFFFFFF : {0}.codePointAt(0);",
                 val
             ));
+            let val = format!("char{i}");
+            js.cx.expose_assert_char();
+            js.prelude(&format!(
+                "if ({val} !== 0xFFFFFF) {{ _assertChar({val}); }}"
+            ));
+            js.push(val);
         }
 
         Instruction::I32FromOptionEnum { hole } => {
diff --git a/crates/cli-support/src/js/mod.rs b/crates/cli-support/src/js/mod.rs
index 9a3899c9266..35bba115fdc 100644
--- a/crates/cli-support/src/js/mod.rs
+++ b/crates/cli-support/src/js/mod.rs
@@ -2116,6 +2116,19 @@ impl<'a> Context<'a> {
         );
     }
 
+    fn expose_assert_char(&mut self) {
+        if !self.should_write_global("assert_char") {
+            return;
+        }
+        self.global(
+            "
+            function _assertChar(c) {
+                if (typeof(c) === 'number' && (c >= 0x110000 || (c >= 0xD800 && c < 0xE000))) throw new Error(`expected a valid Unicode scalar value, found ${c}`);
+            }
+            ",
+        );
+    }
+
     fn expose_make_mut_closure(&mut self) -> Result<(), Error> {
         if !self.should_write_global("make_mut_closure") {
             return Ok(());
diff --git a/src/convert/impls.rs b/src/convert/impls.rs
index 8d2bf4baf6a..4ecef7821ef 100644
--- a/src/convert/impls.rs
+++ b/src/convert/impls.rs
@@ -188,6 +188,7 @@ impl FromWasmAbi for char {
 
     #[inline]
     unsafe fn from_abi(js: u32) -> char {
+        // SAFETY: Checked in bindings.
         char::from_u32_unchecked(js)
     }
 }
diff --git a/tests/wasm/char.js b/tests/wasm/char.js
index d145333e381..b1ee622808f 100644
--- a/tests/wasm/char.js
+++ b/tests/wasm/char.js
@@ -6,6 +6,7 @@ exports.js_identity = a => a;
 exports.js_works = () => {
     assert.strictEqual(wasm.letter(), 'a');
     assert.strictEqual(wasm.face(), '😀');
+    assert.strictEqual(wasm.rust_identity(''), '\u0000');
     assert.strictEqual(wasm.rust_identity('Ղ'), 'Ղ');
     assert.strictEqual(wasm.rust_identity('ҝ'), 'ҝ');
     assert.strictEqual(wasm.rust_identity('Δ'), 'Δ');
@@ -14,4 +15,13 @@ exports.js_works = () => {
     assert.strictEqual(wasm.rust_js_identity('㊻'), '㊻');
     wasm.rust_letter('a');
     wasm.rust_face('😀');
+
+    assert.strictEqual(wasm.rust_option_identity(undefined), undefined);
+    assert.strictEqual(wasm.rust_option_identity(null), undefined);
+    assert.strictEqual(wasm.rust_option_identity(''), '\u0000');
+    assert.strictEqual(wasm.rust_option_identity('\u0000'), '\u0000');
+
+    assert.throws(() => wasm.rust_identity(55357), /c.codePointAt is not a function/);
+    assert.throws(() => wasm.rust_identity('\uD83D'), /expected a valid Unicode scalar value, found 55357/);
+    assert.throws(() => wasm.rust_option_identity('\uD83D'), /expected a valid Unicode scalar value, found 55357/);
 };
diff --git a/tests/wasm/char.rs b/tests/wasm/char.rs
index f852b8d8145..d680a1cae43 100644
--- a/tests/wasm/char.rs
+++ b/tests/wasm/char.rs
@@ -12,6 +12,11 @@ pub fn rust_identity(c: char) -> char {
     c
 }
 
+#[wasm_bindgen]
+pub fn rust_option_identity(c: Option<char>) -> Option<char> {
+    c
+}
+
 #[wasm_bindgen]
 pub fn rust_js_identity(c: char) -> char {
     js_identity(c)