[44.0.0] Merged backports for security advisories (#13007)

* fix(environ): repair unsound StringPool::try_clone()

The 43.0 release introduced a soundness bug in StringPool::try_clone(): the
cloned map retains &'static str keys pointing into the original pool's
strings storage. Once the original Linker is dropped those keys dangle.

Cloning a Linker, then dropping the original one, leaves a linker whose
registered imports could no longer be found, causing instantiation to
fail with "unknown import".

Signed-off-by: Flavio Castelli <fcastelli@suse.com>

* Fix pooling allocator predicate to reset VM permissions

This commit fixes a mistake that was introduced in #9583 where the logic
to reset a linear memory slot in the pooling allocator used the wrong
predicate. Specifically VM permissions must be reset if virtual memory
can be relied on at all, and the preexisting predicate of
`can_elide_bounds_check` was an inaccurate representation of this. The
correct predicate to check is `can_use_virtual_memory`.

* winch: Fix the type of the `table.size` output register

This commit corrects the tagged size of the output of the `table.size`
instruction. Previously this was hardcoded as a 32-bit integer instead
of consulting the table's index type to use the
index-type-sized-register instead.

* winch: Fix a host panic when executing `table.fill`

This commit fixes a possible panic when a Winch-compiled module executes
the `table.fill` instruction. Refactoring in #11254 updated Cranelift
but forgot to update Winch meaning that Winch's indices were still using
the module-level indices instead of the `DefinedTableIndex` space. This
adds some tests and updates Winch's translation to use preexisting
helpers.

* x64: Fix `f64x2.splat` without SSE3

Don't sink a load into `pshufd` which loads 16 bytes, instead force
`put_in_xmm` to ensure only 8 bytes are loaded.

* Properly verify alignment in string transcoding

This commit updates string transcoding between guest modules to properly
verify alignment. Previously alignment was only verified on the first
allocation, not reallocations, which is not spec-compliant. This
additionally fixes a possible host panic when dealing with unaligned
pointers.

* Fix type confusion in AArch64 amode RegScaled folding

* winch: Add add_uextend to perform explicit extension when needed.

This commit fixes an out-of-bounds access caused by the lack zero
extension in the code responsible for calculating the heap address for
loads/stores.

This issue manifests in aarch64 (unlike x64) given that no automatic
extension is performed, resulting in an out-of-bounds access.

An alternative approach is to emit an extend for the index, however
this approach is preferred given that it gives the MacroAssembler
layer better control of how to lower addition, e.g., in aarch64 we can
inline the desired extension in a single instruction.

* winch: Correctly type the result of table.grow

This commit fixes an out-of-bounds access caused by the lack of type
narrowing from the `table.grow` builtin. Without explicit narrowing,
the type is treated as 64-bit value, which could cause issues when
paired with loads/stores.

* Review comments

* Properly handle table index types

Only narrow when dealing with the 64-bit pointer/32-bit tables

* Fix panic with out-of-bounds flags in `Value`

This commit fixes a panic when a component model `Value` is lifted from
a flags value which specifies out-of-bounds bits as 1. This is specified
in the component model to ignore the out-of-bounds bits, which `flags!`
correctly did (and thus `bindgen!`), but `Value` treated out-of-bounds
bits as a panic due to indexing an array.

* Fix bounds checks in FACT's `string_to_compact` method

We need to bounds check the source byte length, not the number of code units.

* Add missing realloc validation in string transcoding

This commit adds a missing validation that a return value of `realloc`
is inbounds during string transcoding. This was accidentally missing on
the transcoding path from `utf8` to `latin1+utf16` which meant that a
nearly-raw pointer could get passed to the host to perform the
transcode.

* winch: Refine zero extension heuristic

This commit refines the zero extension heuristic such that it
unconditionally emits a zero extension when dealing with 32-bit
heaps. This eliminates any ambiguity related to the value of the
memory indices across ISAs.

* Fix failure on 32-bit

* Fix miri test

---------

Signed-off-by: Flavio Castelli <fcastelli@suse.com>
Co-authored-by: Flavio Castelli <fcastelli@suse.com>
Co-authored-by: Shun Kashiwa <shunthedev@gmail.com>
Co-authored-by: Saúl Cabrera <saulecabrera@gmail.com>
Co-authored-by: Nick Fitzgerald <fitzgen@gmail.com>

Author: 5 people

cranelift/codegen/src/isa/aarch64/inst.isle

@@ -3954,11 +3954,11 @@
 ;; Note that this can additionally bundle an extending operation but the
 ;; extension must happen before the shift. This will pattern-match the shift
 ;; first and then if that succeeds afterwards try to find an extend.
-(rule 6 (amode_no_more_iconst ty (iadd _ x (ishl _ y (iconst _ (u64_from_imm64 n)))) offset)
-        (if-let true (u64_eq (ty_bytes ty) (u64_wrapping_shl 1 (shift_masked_imm ty n))))
+(rule 6 (amode_no_more_iconst ty (iadd _ x (ishl shift_ty y (iconst _ (u64_from_imm64 n)))) offset)
+        (if-let true (u64_eq (ty_bytes ty) (u64_wrapping_shl 1 (shift_masked_imm shift_ty n))))
         (amode_reg_scaled (amode_add x offset) y))
-(rule 7 (amode_no_more_iconst ty (iadd _ (ishl _ y (iconst _ (u64_from_imm64 n))) x) offset)
-        (if-let true (u64_eq (ty_bytes ty) (u64_wrapping_shl 1 (shift_masked_imm ty n))))
+(rule 7 (amode_no_more_iconst ty (iadd _ (ishl shift_ty y (iconst _ (u64_from_imm64 n))) x) offset)
+        (if-let true (u64_eq (ty_bytes ty) (u64_wrapping_shl 1 (shift_masked_imm shift_ty n))))
         (amode_reg_scaled (amode_add x offset) y))
 
 (decl amode_reg_scaled (Reg Value) AMode)

cranelift/codegen/src/isa/x64/lower.isle

@@ -4903,7 +4903,7 @@
 (rule 0 (lower (has_type $I64X2 (splat _ src)))
         (x64_pshufd (bitcast_gpr_to_xmm 64 src) 0b01_00_01_00))
 (rule 0 (lower (has_type $F64X2 (splat _ src)))
-        (x64_pshufd src 0b01_00_01_00))
+        (x64_pshufd (put_in_xmm src) 0b01_00_01_00))
 (rule 6 (lower (has_type (multi_lane 64 2) (splat _ (sinkable_load addr))))
         (if-let true (has_sse3))
         (x64_movddup addr))

cranelift/filetests/filetests/isa/aarch64/issue-shift-masked-imm-type.clif

@@ -0,0 +1,95 @@
+test compile precise-output
+set unwind_info=false
+target aarch64
+
+;; Regression test: shift_masked_imm in amode_no_more_iconst must use the ishl
+;; type, not the load access type. When load.i8 has ishl.i64 by 56, the old code
+;; computed shift_masked_imm(I8, 56) = 56 & 7 = 0, incorrectly folding the
+;; shift into a RegScaled amode with LSL #0. The correct masking is
+;; shift_masked_imm(I64, 56) = 56 & 63 = 56, which does not match ty_bytes(I8)
+;; and prevents the fold.
+
+function %load_i8_ishl56_should_not_fold(i64, i64) -> i8 {
+block0(v0: i64, v1: i64):
+  v2 = iconst.i64 56
+  v3 = ishl v1, v2
+  v4 = iadd v0, v3
+  v5 = load.i8 v4
+  return v5
+}
+
+; VCode:
+; block0:
+;   lsl x4, x1, #56
+;   ldrb w0, [x0, x4]
+;   ret
+;
+; Disassembled:
+; block0: ; offset 0x0
+;   lsl x4, x1, #0x38
+;   ldrb w0, [x0, x4] ; trap: heap_oob
+;   ret
+
+function %load_i16_ishl17_should_not_fold(i64, i64) -> i16 {
+block0(v0: i64, v1: i64):
+  v2 = iconst.i64 17
+  v3 = ishl v1, v2
+  v4 = iadd v0, v3
+  v5 = load.i16 v4
+  return v5
+}
+
+; VCode:
+; block0:
+;   lsl x4, x1, #17
+;   ldrh w0, [x0, x4]
+;   ret
+;
+; Disassembled:
+; block0: ; offset 0x0
+;   lsl x4, x1, #0x11
+;   ldrh w0, [x0, x4] ; trap: heap_oob
+;   ret
+
+function %load_i32_ishl34_should_not_fold(i64, i64) -> i32 {
+block0(v0: i64, v1: i64):
+  v2 = iconst.i64 34
+  v3 = ishl v1, v2
+  v4 = iadd v0, v3
+  v5 = load.i32 v4
+  return v5
+}
+
+; VCode:
+; block0:
+;   lsl x4, x1, #34
+;   ldr w0, [x0, x4]
+;   ret
+;
+; Disassembled:
+; block0: ; offset 0x0
+;   lsl x4, x1, #0x22
+;   ldr w0, [x0, x4] ; trap: heap_oob
+;   ret
+
+;; Same as the i8 case but with iadd operands swapped
+function %load_i8_ishl56_swapped_should_not_fold(i64, i64) -> i8 {
+block0(v0: i64, v1: i64):
+  v2 = iconst.i64 56
+  v3 = ishl v1, v2
+  v4 = iadd v3, v0
+  v5 = load.i8 v4
+  return v5
+}
+
+; VCode:
+; block0:
+;   lsl x4, x1, #56
+;   ldrb w0, [x4, x0]
+;   ret
+;
+; Disassembled:
+; block0: ; offset 0x0
+;   lsl x4, x1, #0x38
+;   ldrb w0, [x4, x0] ; trap: heap_oob
+;   ret

crates/environ/src/fact/trampoline.rs

@@ -1764,7 +1764,6 @@ impl<'a, 'b> Compiler<'a, 'b> {
         dst_enc: FE,
     ) -> WasmString<'c> {
         assert!(dst_enc.width() >= src_enc.width());
-        self.validate_string_length(src, dst_enc);
 
         let src_mem_opts = {
             match &src.opts.data_model {
@@ -1779,22 +1778,8 @@ impl<'a, 'b> Compiler<'a, 'b> {
             }
         };
 
-        // Calculate the source byte length given the size of each code
-        // unit. Note that this shouldn't overflow given
-        // `validate_string_length` above.
-        let mut src_byte_len_tmp = None;
-        let src_byte_len = if src_enc.width() == 1 {
-            src.len.idx
-        } else {
-            assert_eq!(src_enc.width(), 2);
-            self.instruction(LocalGet(src.len.idx));
-            self.ptr_uconst(src_mem_opts, 1);
-            self.ptr_shl(src_mem_opts);
-            let tmp = self.local_set_new_tmp(src.opts.data_model.unwrap_memory().ptr());
-            let ret = tmp.idx;
-            src_byte_len_tmp = Some(tmp);
-            ret
-        };
+        let (src_byte_len_tmp, src_byte_len) =
+            self.source_string_byte_len(src, src_enc, src_mem_opts);
 
         // Convert the source code units length to the destination byte
         // length type.
@@ -1856,6 +1841,39 @@ impl<'a, 'b> Compiler<'a, 'b> {
 
         dst
     }
+
+    /// Calculate the source byte length given the size of each code
+    /// unit.
+    ///
+    /// Returns an optional temporary local if it was needed, which the caller
+    /// needs to deallocate with `free_temp_local`. Additionally returns the
+    /// index of the local which contains the byte length of the string, which
+    /// may point to the temporary local passed in.
+    fn source_string_byte_len(
+        &mut self,
+        src: &WasmString<'_>,
+        src_enc: FE,
+        src_mem_opts: &LinearMemoryOptions,
+    ) -> (Option<TempLocal>, u32) {
+        self.validate_string_length(src, src_enc);
+
+        if src_enc.width() == 1 {
+            (None, src.len.idx)
+        } else {
+            assert_eq!(src_enc.width(), 2);
+
+            // Note that this shouldn't overflow given `validate_string_length`
+            // above.
+            self.instruction(LocalGet(src.len.idx));
+            self.ptr_uconst(src_mem_opts, 1);
+            self.ptr_shl(src_mem_opts);
+            let tmp = self.local_set_new_tmp(src.opts.data_model.unwrap_memory().ptr());
+
+            let idx = tmp.idx;
+            (Some(tmp), idx)
+        }
+    }
+
     // Corresponding function for `store_string_to_utf8` in the spec.
     //
     // This translation works by possibly performing a number of
@@ -2133,6 +2151,7 @@ impl<'a, 'b> Compiler<'a, 'b> {
             }
         }));
         self.instruction(LocalSet(dst.ptr.idx));
+        self.verify_aligned(dst_opts.data_model.unwrap_memory(), dst.ptr.idx, 2);
         self.instruction(End); // end of shrink-to-fit
 
         self.free_temp_local(dst_byte_len);
@@ -2227,6 +2246,7 @@ impl<'a, 'b> Compiler<'a, 'b> {
         self.instruction(LocalGet(dst.len.idx)); // new_size
         self.instruction(Call(dst_mem_opts.realloc.unwrap().as_u32()));
         self.instruction(LocalSet(dst.ptr.idx));
+        self.verify_aligned(dst_opts.data_model.unwrap_memory(), dst.ptr.idx, 2);
 
         self.free_temp_local(dst_byte_len);
         self.free_temp_local(src_byte_len);
@@ -2255,7 +2275,9 @@ impl<'a, 'b> Compiler<'a, 'b> {
             DataModel::LinearMemory(opts) => opts,
         };
 
-        self.validate_string_length(src, src_enc);
+        let (src_byte_len_tmp, src_byte_len) =
+            self.source_string_byte_len(src, src_enc, src_mem_opts);
+
         self.convert_src_len_to_dst(src.len.idx, src_mem_opts.ptr(), dst_mem_opts.ptr());
         let dst_len = self.local_tee_new_tmp(dst_mem_opts.ptr());
         let dst_byte_len = self.local_set_new_tmp(dst_mem_opts.ptr());
@@ -2268,7 +2290,7 @@ impl<'a, 'b> Compiler<'a, 'b> {
             }
         };
 
-        self.validate_string_inbounds(src, src.len.idx);
+        self.validate_string_inbounds(src, src_byte_len);
         self.validate_string_inbounds(&dst, dst_byte_len.idx);
 
         // Perform the initial latin1 transcode. This returns the number of
@@ -2308,6 +2330,7 @@ impl<'a, 'b> Compiler<'a, 'b> {
         self.instruction(LocalGet(dst.len.idx)); // new_size
         self.instruction(Call(dst_mem_opts.realloc.unwrap().as_u32()));
         self.instruction(LocalSet(dst.ptr.idx));
+        self.verify_aligned(dst_opts.data_model.unwrap_memory(), dst.ptr.idx, 2);
         self.instruction(End);
 
         // In this block the latin1 encoding failed. The host transcode
@@ -2333,6 +2356,8 @@ impl<'a, 'b> Compiler<'a, 'b> {
         self.instruction(LocalTee(dst_byte_len.idx));
         self.instruction(Call(dst_mem_opts.realloc.unwrap().as_u32()));
         self.instruction(LocalSet(dst.ptr.idx));
+        self.verify_aligned(dst_opts.data_model.unwrap_memory(), dst.ptr.idx, 2);
+        self.validate_string_inbounds(&dst, dst_byte_len.idx);
 
         // Call the host utf16 transcoding function. This will inflate the
         // prior latin1 bytes and then encode the rest of the source string
@@ -2372,6 +2397,7 @@ impl<'a, 'b> Compiler<'a, 'b> {
         self.ptr_shl(dst_mem_opts);
         self.instruction(Call(dst_mem_opts.realloc.unwrap().as_u32()));
         self.instruction(LocalSet(dst.ptr.idx));
+        self.verify_aligned(dst_opts.data_model.unwrap_memory(), dst.ptr.idx, 2);
         self.instruction(End);
 
         // Tag the returned pointer as utf16
@@ -2384,6 +2410,9 @@ impl<'a, 'b> Compiler<'a, 'b> {
 
         self.free_temp_local(src_len_tmp);
         self.free_temp_local(dst_byte_len);
+        if let Some(tmp) = src_byte_len_tmp {
+            self.free_temp_local(tmp);
+        }
 
         dst
     }

crates/environ/src/string_pool.rs

@@ -73,10 +73,20 @@ impl fmt::Debug for StringPool {
 
 impl TryClone for StringPool {
     fn try_clone(&self) -> Result<Self, OutOfMemory> {
-        Ok(StringPool {
-            map: self.map.try_clone()?,
-            strings: self.strings.try_clone()?,
-        })
+        let mut new_pool = StringPool::new();
+        // Re-intern strings in index order so that each Atom value is
+        // identical in the clone — callers that hold Atoms from the original
+        // can use them interchangeably with the clone.
+        //
+        // Directly cloning `self.map` would copy &'static str keys that point
+        // into the *original* pool's `strings` allocation. Those pointers
+        // become dangling once the original is dropped, leading to UB on any
+        // subsequent lookup. Re-interning ensures the cloned map's keys point
+        // into the clone's own `strings`.
+        for s in self.strings.iter() {
+            new_pool.insert(s)?;
+        }
+        Ok(new_pool)
     }
 }
 

crates/wasmtime/src/runtime/component/values.rs

@@ -1142,7 +1142,7 @@ fn lower_map<T>(
 }
 
 fn push_flags(ty: &TypeFlags, flags: &mut Vec<String>, mut offset: u32, mut bits: u32) {
-    while bits > 0 {
+    while bits > 0 && usize::try_from(offset).unwrap() < ty.names.len() {
         if bits & 1 != 0 {
             flags.push(ty.names[offset as usize].clone());
         }

crates/wasmtime/src/runtime/vm/cow.rs

@@ -461,7 +461,7 @@ impl MemoryImageSlot {
         let host_page_size_log2 = u8::try_from(host_page_size().ilog2()).unwrap();
         if initial_size_bytes_page_aligned < self.accessible
             && (tunables.memory_guard_size > 0
-                || ty.can_elide_bounds_check(tunables, host_page_size_log2))
+                || ty.can_use_virtual_memory(tunables, host_page_size_log2))
         {
             self.set_protection(initial_size_bytes_page_aligned..self.accessible, false)?;
             self.accessible = initial_size_bytes_page_aligned;

tests/all/component_model/dynamic.rs

@@ -1237,3 +1237,72 @@ fn introspection() -> Result<()> {
     );
     Ok(())
 }
+
+#[test]
+fn flags_beyond_end() -> Result<()> {
+    let engine = super::engine();
+
+    let component = Component::new(
+        &engine,
+        r#"
+            (component
+                (type $f' (flags "a" "b"))
+                (export $f "f" (type $f'))
+
+                (core module $m
+                    (func (export "r") (param i32) (result i32) local.get 0)
+                )
+                (core instance $i (instantiate $m))
+                (func (export "run") (param "x" u32) (result $f)
+                    (canon lift (core func $i "r")))
+            )
+        "#,
+    )?;
+    let mut store = Store::new(&engine, ());
+    let instance = Linker::new(&engine).instantiate(&mut store, &component)?;
+    let run = instance.get_func(&mut store, "run").unwrap();
+
+    let mut output = [Val::Bool(false)];
+    run.call(&mut store, &[Val::U32(0)], &mut output)?;
+    assert_eq!(output[0], Val::Flags(vec![]));
+
+    let mut output = [Val::Bool(false)];
+    run.call(&mut store, &[Val::U32(1)], &mut output)?;
+    assert_eq!(output[0], Val::Flags(vec!["a".to_string()]));
+
+    let mut output = [Val::Bool(false)];
+    run.call(&mut store, &[Val::U32(2)], &mut output)?;
+    assert_eq!(output[0], Val::Flags(vec!["b".to_string()]));
+
+    let mut output = [Val::Bool(false)];
+    run.call(&mut store, &[Val::U32(3)], &mut output)?;
+    assert_eq!(
+        output[0],
+        Val::Flags(vec!["a".to_string(), "b".to_string()])
+    );
+    let mut output = [Val::Bool(false)];
+
+    run.call(&mut store, &[Val::U32(4)], &mut output)?;
+    assert_eq!(output[0], Val::Flags(vec![]));
+
+    run.call(&mut store, &[Val::U32(5)], &mut output)?;
+    assert_eq!(output[0], Val::Flags(vec!["a".to_string()]));
+
+    wasmtime::component::flags! {
+        F {
+            #[component(name = "a")]
+            const A;
+            #[component(name = "b")]
+            const B;
+        }
+    }
+
+    let run = instance.get_typed_func::<(u32,), (F,)>(&mut store, "run")?;
+    assert_eq!(run.call(&mut store, (0,))?, (F::empty(),));
+    assert_eq!(run.call(&mut store, (1,))?, (F::A,));
+    assert_eq!(run.call(&mut store, (2,))?, (F::B,));
+    assert_eq!(run.call(&mut store, (3,))?, (F::A | F::B,));
+    assert_eq!(run.call(&mut store, (4,))?, (F::empty(),));
+    assert_eq!(run.call(&mut store, (5,))?, (F::A,));
+    Ok(())
+}