Skip to content

Commit

Permalink
Fix character boundary issues in preview1 host adapter (#7011)
Browse files Browse the repository at this point in the history
* Fix character boundary issues in preview1 host adapter

This fixes two separate issues in the preview1-to-preview2 host-side
adapter in the `wasmtime-wasi` crate. Both instances were copying a
truncated string to the guest program but the truncation was happening
in the middle of a unicode character so truncation now happens on bytes
instead of a string.

* Update `write_bytes` to take `&[u8]`

Try to weed out any accidental string-related issues for the future.
  • Loading branch information
alexcrichton authored Sep 20, 2023
1 parent f2b43d8 commit 04b09c8
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 13 deletions.
35 changes: 35 additions & 0 deletions crates/test-programs/wasi-tests/src/bin/fd_readdir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,40 @@ unsafe fn test_fd_readdir_lots(dir_fd: wasi::Fd) {
}
}

unsafe fn test_fd_readdir_unicode_boundary(dir_fd: wasi::Fd) {
let filename = "Действие";
let file_fd = wasi::path_open(
dir_fd,
0,
filename,
wasi::OFLAGS_CREAT,
wasi::RIGHTS_FD_READ | wasi::RIGHTS_FD_WRITE,
0,
0,
)
.expect("failed to create file");
assert!(
file_fd > libc::STDERR_FILENO as wasi::Fd,
"file descriptor range check",
);
wasi::fd_close(file_fd).expect("closing a file");

let mut buf = Vec::new();
'outer: loop {
let len = wasi::fd_readdir(dir_fd, buf.as_mut_ptr(), buf.capacity(), 0).unwrap();
buf.set_len(len);

for entry in ReadDir::from_slice(&buf) {
if entry.name == filename {
break 'outer;
}
}
buf = Vec::with_capacity(buf.capacity() + 1);
}

wasi::path_unlink_file(dir_fd, filename).expect("removing a file");
}

fn main() {
let mut args = env::args();
let prog = args.next().unwrap();
Expand All @@ -227,4 +261,5 @@ fn main() {
// Run the tests.
unsafe { test_fd_readdir(dir_fd) }
unsafe { test_fd_readdir_lots(dir_fd) }
unsafe { test_fd_readdir_unicode_boundary(dir_fd) }
}
25 changes: 25 additions & 0 deletions crates/test-programs/wasi-tests/src/bin/readlink.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,30 @@ unsafe fn test_readlink(dir_fd: wasi::Fd) {
wasi::path_unlink_file(dir_fd, "symlink").expect("removing a file");
}

unsafe fn test_incremental_readlink(dir_fd: wasi::Fd) {
let filename = "Действие";
create_file(dir_fd, filename);

wasi::path_symlink(filename, dir_fd, "symlink").expect("creating a symlink");

let mut buf = Vec::new();
loop {
if buf.capacity() > 2 * filename.len() {
panic!()
}
let bufused = wasi::path_readlink(dir_fd, "symlink", buf.as_mut_ptr(), buf.capacity())
.expect("readlink should succeed");
buf.set_len(bufused);
if buf.capacity() > filename.len() {
assert!(buf.starts_with(filename.as_bytes()));
break;
}
buf = Vec::with_capacity(buf.capacity() + 1);
}
wasi::path_unlink_file(dir_fd, filename).expect("removing a file");
wasi::path_unlink_file(dir_fd, "symlink").expect("removing a file");
}

fn main() {
let mut args = env::args();
let prog = args.next().unwrap();
Expand All @@ -53,4 +77,5 @@ fn main() {

// Run the tests.
unsafe { test_readlink(dir_fd) }
unsafe { test_incremental_readlink(dir_fd) }
}
31 changes: 18 additions & 13 deletions crates/wasi/src/preview2/preview1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -779,7 +779,7 @@ type Result<T, E = types::Error> = std::result::Result<T, E>;

fn write_bytes<'a>(
ptr: impl Borrow<GuestPtr<'a, u8>>,
buf: impl AsRef<[u8]>,
buf: &[u8],
) -> Result<GuestPtr<'a, u8>, types::Error> {
// NOTE: legacy implementation always returns Inval errno

Expand Down Expand Up @@ -870,7 +870,7 @@ impl<
argv.write(argv_buf)?;
let argv = argv.add(1)?;

let argv_buf = write_bytes(argv_buf, arg)?;
let argv_buf = write_bytes(argv_buf, arg.as_bytes())?;
let argv_buf = write_byte(argv_buf, 0)?;

Ok((argv, argv_buf))
Expand Down Expand Up @@ -910,9 +910,9 @@ impl<
environ.write(environ_buf)?;
let environ = environ.add(1)?;

let environ_buf = write_bytes(environ_buf, k)?;
let environ_buf = write_bytes(environ_buf, k.as_bytes())?;
let environ_buf = write_byte(environ_buf, b'=')?;
let environ_buf = write_bytes(environ_buf, v)?;
let environ_buf = write_bytes(environ_buf, v.as_bytes())?;
let environ_buf = write_byte(environ_buf, 0)?;

Ok((environ, environ_buf))
Expand Down Expand Up @@ -1514,7 +1514,7 @@ impl<
if p.len() > path_max_len {
return Err(types::Errno::Nametoolong.into());
}
write_bytes(path, p)?;
write_bytes(path, p.as_bytes())?;
return Ok(());
}
Err(types::Errno::Notdir.into()) // NOTE: legacy implementation returns NOTDIR here
Expand Down Expand Up @@ -1672,7 +1672,8 @@ impl<
);
let mut buf = *buf;
let mut cap = buf_len;
for (ref entry, mut path) in head.into_iter().chain(dir.into_iter()).skip(cookie) {
for (ref entry, path) in head.into_iter().chain(dir.into_iter()).skip(cookie) {
let mut path = path.into_bytes();
assert_eq!(
1,
size_of_val(&entry.d_type),
Expand All @@ -1692,7 +1693,7 @@ impl<
path.truncate(cap);
}
cap = cap.checked_sub(path.len() as _).unwrap();
buf = write_bytes(buf, path)?;
buf = write_bytes(buf, &path)?;
if cap == 0 {
return Ok(buf_len);
}
Expand Down Expand Up @@ -1901,11 +1902,15 @@ impl<
) -> Result<types::Size, types::Error> {
let dirfd = self.get_dir_fd(dirfd)?;
let path = read_string(path)?;
let mut path = self.readlink_at(dirfd, path).await.map_err(|e| {
e.try_into()
.context("failed to call `readlink-at`")
.unwrap_or_else(types::Error::trap)
})?;
let mut path = self
.readlink_at(dirfd, path)
.await
.map_err(|e| {
e.try_into()
.context("failed to call `readlink-at`")
.unwrap_or_else(types::Error::trap)
})?
.into_bytes();
if let Ok(buf_len) = buf_len.try_into() {
// `path` cannot be longer than `usize`, only truncate if `buf_len` fits in `usize`
path.truncate(buf_len);
Expand Down Expand Up @@ -2031,7 +2036,7 @@ impl<
.get_random_bytes(buf_len.into())
.context("failed to call `get-random-bytes`")
.map_err(types::Error::trap)?;
write_bytes(buf, rand)?;
write_bytes(buf, &rand)?;
Ok(())
}

Expand Down

0 comments on commit 04b09c8

Please sign in to comment.