fix(wasip1): fd_renumber panic in the host (#11276)

* fix(wasip1): prevent duplicate FD usage

The implementation assumed that only the runtime could ever issue FDs,
however that's not the case in p1, where guests can choose arbitrary
FDs to use (e.g. via `fd_renumber`).

Due to incorrect accounting, guests could "mark" arbitrary FDs as "free"
and trigger a panic in the host by requesting a new FD.

Signed-off-by: Roman Volosatovs <[email protected]>

* test(wasip1): expand `fd_renumber` test

Signed-off-by: Roman Volosatovs <[email protected]>

* doc: add release notes

Signed-off-by: Roman Volosatovs <[email protected]>

* test(wasip1): ignore `fd_renumber` tests when using adapter

Signed-off-by: Roman Volosatovs <[email protected]>

* refactor(wasip1): do not modify descriptors on `fd_renumber(n, n)`

Since `remove` is now only used once, remove it.

As a sideffect, this makes the implementation more explicit .

Signed-off-by: Roman Volosatovs <[email protected]>

* doc: reference the CVE

prtest:full

Signed-off-by: Roman Volosatovs <[email protected]>

---------

Signed-off-by: Roman Volosatovs <[email protected]>

Author: rvolosatovs

RELEASES.md

@@ -6,6 +6,11 @@ Unreleased.
 
 ### Changed
 
+### Fixed
+
+* Fix a panic in the host caused by preview1 guests using `fd_renumber`.
+  [CVE-2025-53901](https://github.com/bytecodealliance/wasmtime/security/advisories/GHSA-fm79-3f68-h2fc).
+
 --------------------------------------------------------------------------------
 
 Release notes for previous releases of Wasmtime can be found on the respective

crates/test-programs/src/bin/preview1_renumber.rs

@@ -74,6 +74,38 @@ unsafe fn test_renumber(dir_fd: wasip1::Fd) {
     );
 
     wasip1::fd_close(fd_to).expect("closing a file");
+
+    wasip1::fd_renumber(0, 0).expect("renumbering 0 to 0");
+    let fd_file3 = wasip1::path_open(
+        dir_fd,
+        0,
+        "file3",
+        wasip1::OFLAGS_CREAT,
+        wasip1::RIGHTS_FD_READ | wasip1::RIGHTS_FD_WRITE,
+        0,
+        0,
+    )
+    .expect("opening a file");
+    assert!(
+        fd_file3 > libc::STDERR_FILENO as wasip1::Fd,
+        "file descriptor range check",
+    );
+
+    wasip1::fd_renumber(fd_file3, u32::MAX).expect("renumbering file FD to `u32::MAX`");
+    let fd_file4 = wasip1::path_open(
+        dir_fd,
+        0,
+        "file4",
+        wasip1::OFLAGS_CREAT,
+        wasip1::RIGHTS_FD_READ | wasip1::RIGHTS_FD_WRITE,
+        0,
+        0,
+    )
+    .expect("opening a file");
+    assert!(
+        fd_file4 > libc::STDERR_FILENO as wasip1::Fd,
+        "file descriptor range check",
+    );
 }
 
 fn main() {

crates/wasi/src/preview1.rs

@@ -74,9 +74,8 @@ use crate::p2::bindings::{
 };
 use crate::p2::{FsError, IsATTY, WasiCtx, WasiImpl, WasiView};
 use anyhow::{Context, bail};
-use std::collections::{BTreeMap, HashSet};
+use std::collections::{BTreeMap, BTreeSet, HashSet, btree_map};
 use std::mem::{self, size_of, size_of_val};
-use std::ops::{Deref, DerefMut};
 use std::slice;
 use std::sync::Arc;
 use std::sync::atomic::{AtomicU64, Ordering};
@@ -286,21 +285,7 @@ struct WasiPreview1Adapter {
 #[derive(Debug, Default)]
 struct Descriptors {
     used: BTreeMap<u32, Descriptor>,
-    free: Vec<u32>,
-}
-
-impl Deref for Descriptors {
-    type Target = BTreeMap<u32, Descriptor>;
-
-    fn deref(&self) -> &Self::Target {
-        &self.used
-    }
-}
-
-impl DerefMut for Descriptors {
-    fn deref_mut(&mut self) -> &mut Self::Target {
-        &mut self.used
-    }
+    free: BTreeSet<u32>,
 }
 
 impl Descriptors {
@@ -377,42 +362,34 @@ impl Descriptors {
 
     /// Returns next descriptor number, which was never assigned
     fn unused(&self) -> Result<u32> {
-        match self.last_key_value() {
+        match self.used.last_key_value() {
             Some((fd, _)) => {
                 if let Some(fd) = fd.checked_add(1) {
                     return Ok(fd);
                 }
-                if self.len() == u32::MAX as usize {
+                if self.used.len() == u32::MAX as usize {
                     return Err(types::Errno::Loop.into());
                 }
                 // TODO: Optimize
                 Ok((0..u32::MAX)
                     .rev()
-                    .find(|fd| !self.contains_key(fd))
+                    .find(|fd| !self.used.contains_key(fd))
                     .expect("failed to find an unused file descriptor"))
             }
             None => Ok(0),
         }
     }
 
-    /// Removes the [Descriptor] corresponding to `fd`
-    fn remove(&mut self, fd: types::Fd) -> Option<Descriptor> {
-        let fd = fd.into();
-        let desc = self.used.remove(&fd)?;
-        self.free.push(fd);
-        Some(desc)
-    }
-
     /// Pushes the [Descriptor] returning corresponding number.
     /// This operation will try to reuse numbers previously removed via [`Self::remove`]
     /// and rely on [`Self::unused`] if no free numbers are recorded
     fn push(&mut self, desc: Descriptor) -> Result<u32> {
-        let fd = if let Some(fd) = self.free.pop() {
+        let fd = if let Some(fd) = self.free.pop_last() {
             fd
         } else {
             self.unused()?
         };
-        assert!(self.insert(fd, desc).is_none());
+        assert!(self.used.insert(fd, desc).is_none());
         Ok(fd)
     }
 }
@@ -453,15 +430,15 @@ impl Transaction<'_> {
     /// Returns [`types::Errno::Badf`] if no [`Descriptor`] is found
     fn get_descriptor(&self, fd: types::Fd) -> Result<&Descriptor> {
         let fd = fd.into();
-        let desc = self.descriptors.get(&fd).ok_or(types::Errno::Badf)?;
+        let desc = self.descriptors.used.get(&fd).ok_or(types::Errno::Badf)?;
         Ok(desc)
     }
 
     /// Borrows [`File`] corresponding to `fd`
     /// if it describes a [`Descriptor::File`]
     fn get_file(&self, fd: types::Fd) -> Result<&File> {
         let fd = fd.into();
-        match self.descriptors.get(&fd) {
+        match self.descriptors.used.get(&fd) {
             Some(Descriptor::File(file)) => Ok(file),
             _ => Err(types::Errno::Badf.into()),
         }
@@ -471,7 +448,7 @@ impl Transaction<'_> {
     /// if it describes a [`Descriptor::File`]
     fn get_file_mut(&mut self, fd: types::Fd) -> Result<&mut File> {
         let fd = fd.into();
-        match self.descriptors.get_mut(&fd) {
+        match self.descriptors.used.get_mut(&fd) {
             Some(Descriptor::File(file)) => Ok(file),
             _ => Err(types::Errno::Badf.into()),
         }
@@ -485,7 +462,7 @@ impl Transaction<'_> {
     /// Returns [`types::Errno::Spipe`] if the descriptor corresponds to stdio
     fn get_seekable(&self, fd: types::Fd) -> Result<&File> {
         let fd = fd.into();
-        match self.descriptors.get(&fd) {
+        match self.descriptors.used.get(&fd) {
             Some(Descriptor::File(file)) => Ok(file),
             Some(
                 Descriptor::Stdin { .. } | Descriptor::Stdout { .. } | Descriptor::Stderr { .. },
@@ -518,7 +495,7 @@ impl Transaction<'_> {
     /// if it describes a [`Descriptor::Directory`]
     fn get_dir_fd(&self, fd: types::Fd) -> Result<Resource<filesystem::Descriptor>> {
         let fd = fd.into();
-        match self.descriptors.get(&fd) {
+        match self.descriptors.used.get(&fd) {
             Some(Descriptor::Directory { fd, .. }) => Ok(fd.borrowed()),
             _ => Err(types::Errno::Badf.into()),
         }
@@ -1327,11 +1304,13 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiP1Ctx {
         _memory: &mut GuestMemory<'_>,
         fd: types::Fd,
     ) -> Result<(), types::Error> {
-        let desc = self
-            .transact()?
-            .descriptors
-            .remove(fd)
-            .ok_or(types::Errno::Badf)?;
+        let desc = {
+            let fd = fd.into();
+            let mut st = self.transact()?;
+            let desc = st.descriptors.used.remove(&fd).ok_or(types::Errno::Badf)?;
+            st.descriptors.free.insert(fd);
+            desc
+        };
         match desc {
             Descriptor::Stdin { stream, .. } => {
                 streams::HostInputStream::drop(&mut self.as_io_impl(), stream)
@@ -1830,8 +1809,17 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiP1Ctx {
         to: types::Fd,
     ) -> Result<(), types::Error> {
         let mut st = self.transact()?;
-        let desc = st.descriptors.remove(from).ok_or(types::Errno::Badf)?;
-        st.descriptors.insert(to.into(), desc);
+        let from = from.into();
+        let btree_map::Entry::Occupied(desc) = st.descriptors.used.entry(from) else {
+            return Err(types::Errno::Badf.into());
+        };
+        let to = to.into();
+        if from != to {
+            let desc = desc.remove();
+            st.descriptors.free.insert(from);
+            st.descriptors.free.remove(&to);
+            st.descriptors.used.insert(to, desc);
+        }
         Ok(())
     }
 

crates/wasi/tests/all/p2/async_.rs

@@ -231,6 +231,7 @@ async fn preview1_remove_nonempty_directory() {
         .await
         .unwrap()
 }
+#[ignore = "panics"]
 #[test_log::test(tokio::test(flavor = "multi_thread"))]
 async fn preview1_renumber() {
     run(PREVIEW1_RENUMBER_COMPONENT, false).await.unwrap()

crates/wasi/tests/all/p2/sync.rs

@@ -187,6 +187,7 @@ fn preview1_remove_directory() {
 fn preview1_remove_nonempty_directory() {
     run(PREVIEW1_REMOVE_NONEMPTY_DIRECTORY_COMPONENT, false).unwrap()
 }
+#[ignore = "panics"]
 #[test_log::test]
 fn preview1_renumber() {
     run(PREVIEW1_RENUMBER_COMPONENT, false).unwrap()