From 7456336b8ebb6223e913a61ac628f310981661df Mon Sep 17 00:00:00 2001 From: Roman Volosatovs Date: Mon, 14 Jul 2025 19:05:52 +0200 Subject: [PATCH 1/7] 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 --- crates/wasi/src/preview1.rs | 46 ++++++++++++++----------------------- 1 file changed, 17 insertions(+), 29 deletions(-) diff --git a/crates/wasi/src/preview1.rs b/crates/wasi/src/preview1.rs index 3285ff45b25b..c90002326a70 100644 --- a/crates/wasi/src/preview1.rs +++ b/crates/wasi/src/preview1.rs @@ -74,9 +74,8 @@ use crate::p2::bindings::{ use crate::p2::{FsError, IsATTY, WasiCtx, WasiImpl, WasiView}; use crate::ResourceTable; use anyhow::{bail, Context}; -use std::collections::{BTreeMap, HashSet}; +use std::collections::{BTreeMap, BTreeSet, HashSet}; use std::mem::{self, size_of, size_of_val}; -use std::ops::{Deref, DerefMut}; use std::slice; use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::Arc; @@ -286,21 +285,7 @@ struct WasiPreview1Adapter { #[derive(Debug, Default)] struct Descriptors { used: BTreeMap, - free: Vec, -} - -impl Deref for Descriptors { - type Target = BTreeMap; - - fn deref(&self) -> &Self::Target { - &self.used - } -} - -impl DerefMut for Descriptors { - fn deref_mut(&mut self) -> &mut Self::Target { - &mut self.used - } + free: BTreeSet, } impl Descriptors { @@ -377,18 +362,18 @@ impl Descriptors { /// Returns next descriptor number, which was never assigned fn unused(&self) -> Result { - 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), @@ -399,7 +384,7 @@ impl Descriptors { fn remove(&mut self, fd: types::Fd) -> Option { let fd = fd.into(); let desc = self.used.remove(&fd)?; - self.free.push(fd); + self.free.insert(fd); Some(desc) } @@ -407,12 +392,12 @@ impl Descriptors { /// 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 { - 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,7 +438,7 @@ 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) } @@ -461,7 +446,7 @@ impl Transaction<'_> { /// 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 +456,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 +470,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 +503,7 @@ impl Transaction<'_> { /// if it describes a [`Descriptor::Directory`] fn get_dir_fd(&self, fd: types::Fd) -> Result> { 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()), } @@ -1832,7 +1817,10 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiP1Ctx { ) -> 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 to = to.into(); + st.descriptors.free.remove(&to); + st.descriptors.used.insert(to, desc); Ok(()) } From 33f71bde856d9fa7ae5b76081a44bbd34aae99d6 Mon Sep 17 00:00:00 2001 From: Roman Volosatovs Date: Mon, 14 Jul 2025 19:23:02 +0200 Subject: [PATCH 2/7] test(wasip1): expand `fd_renumber` test Signed-off-by: Roman Volosatovs --- .../src/bin/preview1_renumber.rs | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/crates/test-programs/src/bin/preview1_renumber.rs b/crates/test-programs/src/bin/preview1_renumber.rs index bfeb250b85cf..ec3a0a2e2362 100644 --- a/crates/test-programs/src/bin/preview1_renumber.rs +++ b/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() { From 101778c366c8a90db616748213d7a61b368c21cd Mon Sep 17 00:00:00 2001 From: Roman Volosatovs Date: Mon, 14 Jul 2025 20:00:11 +0200 Subject: [PATCH 3/7] 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 --- crates/wasi/src/preview1.rs | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/crates/wasi/src/preview1.rs b/crates/wasi/src/preview1.rs index c90002326a70..b2db02cb3ce0 100644 --- a/crates/wasi/src/preview1.rs +++ b/crates/wasi/src/preview1.rs @@ -74,7 +74,7 @@ use crate::p2::bindings::{ use crate::p2::{FsError, IsATTY, WasiCtx, WasiImpl, WasiView}; use crate::ResourceTable; use anyhow::{bail, Context}; -use std::collections::{BTreeMap, BTreeSet, HashSet}; +use std::collections::{btree_map, BTreeMap, BTreeSet, HashSet}; use std::mem::{self, size_of, size_of_val}; use std::slice; use std::sync::atomic::{AtomicU64, Ordering}; @@ -380,14 +380,6 @@ impl Descriptors { } } - /// Removes the [Descriptor] corresponding to `fd` - fn remove(&mut self, fd: types::Fd) -> Option { - let fd = fd.into(); - let desc = self.used.remove(&fd)?; - self.free.insert(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 @@ -1313,11 +1305,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) @@ -1816,11 +1810,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)?; - + 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(); - st.descriptors.free.remove(&to); - st.descriptors.used.insert(to, desc); + if from != to { + let desc = desc.remove(); + st.descriptors.free.insert(from); + st.descriptors.free.remove(&to); + st.descriptors.used.insert(to, desc); + } Ok(()) } From f9c494e90d741dddfec1ba0267c9dc601688a2a5 Mon Sep 17 00:00:00 2001 From: Roman Volosatovs Date: Mon, 14 Jul 2025 20:12:54 +0200 Subject: [PATCH 4/7] fix(wasip1-adapter): prevent `unreachable` panic on `fd_renumber` Signed-off-by: Roman Volosatovs --- .../src/bin/preview1_renumber.rs | 10 ++++++- .../src/descriptors.rs | 30 +++++++++++-------- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/crates/test-programs/src/bin/preview1_renumber.rs b/crates/test-programs/src/bin/preview1_renumber.rs index ec3a0a2e2362..394e5776f4f4 100644 --- a/crates/test-programs/src/bin/preview1_renumber.rs +++ b/crates/test-programs/src/bin/preview1_renumber.rs @@ -91,7 +91,15 @@ unsafe fn test_renumber(dir_fd: wasip1::Fd) { "file descriptor range check", ); - wasip1::fd_renumber(fd_file3, u32::MAX).expect("renumbering file FD to `u32::MAX`"); + wasip1::fd_renumber(fd_file3, 127).expect("renumbering FD to 127"); + match wasip1::fd_renumber(127, u32::MAX) { + Err(wasip1::ERRNO_NOMEM) => { + // The preview1 adapter cannot handle more than 128 descriptors + eprintln!("fd_renumber({fd_file3}, {}) returned NOMEM", u32::MAX) + } + res => res.expect("renumbering FD to `u32::MAX`"), + } + let fd_file4 = wasip1::path_open( dir_fd, 0, diff --git a/crates/wasi-preview1-component-adapter/src/descriptors.rs b/crates/wasi-preview1-component-adapter/src/descriptors.rs index 5dd284d3c94a..940effacaf91 100644 --- a/crates/wasi-preview1-component-adapter/src/descriptors.rs +++ b/crates/wasi-preview1-component-adapter/src/descriptors.rs @@ -330,8 +330,8 @@ impl Descriptors { .ok_or(wasi::ERRNO_BADF) } - // Internal: close a fd, returning the descriptor. - fn close_(&mut self, fd: Fd) -> Result { + // Close an fd. + pub fn close(&mut self, fd: Fd) -> Result<(), Errno> { // Throw an error if closing an fd which is already closed match self.get(fd)? { Descriptor::Closed(_) => Err(wasi::ERRNO_BADF)?, @@ -341,12 +341,7 @@ impl Descriptors { let last_closed = self.closed; let prev = std::mem::replace(self.get_mut(fd)?, Descriptor::Closed(last_closed)); self.closed = Some(fd); - Ok(prev) - } - - // Close an fd. - pub fn close(&mut self, fd: Fd) -> Result<(), Errno> { - drop(self.close_(fd)?); + drop(prev); Ok(()) } @@ -366,11 +361,20 @@ impl Descriptors { while self.table_len.get() as u32 <= to_fd { self.push_closed()?; } - // Then, close from_fd and put its contents into to_fd: - let desc = self.close_(from_fd)?; - // TODO FIXME if this overwrites a preopen, do we need to clear it from the preopen table? - *self.get_mut(to_fd)? = desc; - + // Throw an error if renumbering a closed fd + match self.get(from_fd)? { + Descriptor::Closed(_) => Err(wasi::ERRNO_BADF)?, + _ => {} + } + // Close from_fd and put its contents into to_fd + if from_fd != to_fd { + // Mutate the descriptor to be closed, and push the closed fd onto the head of the linked list: + let last_closed = self.closed; + let desc = std::mem::replace(self.get_mut(from_fd)?, Descriptor::Closed(last_closed)); + self.closed = Some(from_fd); + // TODO FIXME if this overwrites a preopen, do we need to clear it from the preopen table? + *self.get_mut(to_fd)? = desc; + } Ok(()) } From 1057d8a34dcc5e49c128f026f41f831dd0838a48 Mon Sep 17 00:00:00 2001 From: Roman Volosatovs Date: Mon, 14 Jul 2025 14:44:58 +0200 Subject: [PATCH 5/7] doc: add release notes Signed-off-by: Roman Volosatovs --- RELEASES.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/RELEASES.md b/RELEASES.md index 666052e7d606..427a55a5dafd 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -1,3 +1,14 @@ +## 33.0.2 + +Released 2025-07-18. + +### Fixed + +* Fix a panic in the host caused by preview1 guests using `fd_renumber`. + [GHSA-fm79-3f68-h2fc](https://github.com/bytecodealliance/wasmtime/security/advisories/GHSA-fm79-3f68-h2fc). + +* Fix a panic in the preview1 adapter caused by guests using `fd_renumber`. + ## 33.0.1 Released 2025-06-24. From 950473872048d723eaf8ea98f52f1d447bdc8f80 Mon Sep 17 00:00:00 2001 From: Roman Volosatovs Date: Fri, 18 Jul 2025 13:15:29 +0200 Subject: [PATCH 6/7] doc: reference the CVE prtest:full Signed-off-by: Roman Volosatovs --- RELEASES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RELEASES.md b/RELEASES.md index 427a55a5dafd..e339b9d0ee17 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -5,7 +5,7 @@ Released 2025-07-18. ### Fixed * Fix a panic in the host caused by preview1 guests using `fd_renumber`. - [GHSA-fm79-3f68-h2fc](https://github.com/bytecodealliance/wasmtime/security/advisories/GHSA-fm79-3f68-h2fc). + [CVE-2025-53901](https://github.com/bytecodealliance/wasmtime/security/advisories/GHSA-fm79-3f68-h2fc). * Fix a panic in the preview1 adapter caused by guests using `fd_renumber`. From 74a6761a46f17e3f68914627094311a3a97e8fc0 Mon Sep 17 00:00:00 2001 From: Roman Volosatovs Date: Fri, 18 Jul 2025 13:42:49 +0200 Subject: [PATCH 7/7] doc: add PR reference prtest:full Signed-off-by: Roman Volosatovs --- RELEASES.md | 1 + 1 file changed, 1 insertion(+) diff --git a/RELEASES.md b/RELEASES.md index e339b9d0ee17..83d936a29bb0 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -8,6 +8,7 @@ Released 2025-07-18. [CVE-2025-53901](https://github.com/bytecodealliance/wasmtime/security/advisories/GHSA-fm79-3f68-h2fc). * Fix a panic in the preview1 adapter caused by guests using `fd_renumber`. + [#11277](https://github.com/bytecodealliance/wasmtime/pull/11277) ## 33.0.1