Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Running *.wasm files directly doesn't sandbox the filesystem #4267

Closed
yagehu opened this issue Oct 21, 2023 · 11 comments
Closed

Running *.wasm files directly doesn't sandbox the filesystem #4267

yagehu opened this issue Oct 21, 2023 · 11 comments
Assignees
Labels
bug Something isn't working 📦 lib-vfs About wasmer-vfs needs-tests priority-high High priority issue
Milestone

Comments

@yagehu
Copy link
Contributor

yagehu commented Oct 21, 2023

In wasmtime or other Wasm runtimes with WASI support, you have to explicit preopen a directory to have access to the host filesystem. With wasmer, it seems the cwd is opened by default.

For example, given this small Rust program:

fn main() {
    let f = std::fs::OpenOptions::new()
        .write(true)
        .create_new(true)
        .open("abc")
        .unwrap();
}

Running it as wasmer run test.wasm will just create a file named abc in cwd. Is it possible to disable this behavior. I only want host fs access if I explicitly enable it.

The above program is compiled with cargo build --target wasm32-wasi.

@yagehu yagehu added the ❓ question I've a question! label Oct 21, 2023
@Arshia001
Copy link
Member

Hey @yagehu!

Are you sure you're not mounting the directory via --mapdir /:.? AFAIK, no directories are mounted automatically when running wasmer without --mapdir or --dir.

@Arshia001 Arshia001 added 📦 lib-vfs About wasmer-vfs priority-medium Medium priority issue ❔ needs more context Needs more context and information from the issue poster. labels Oct 24, 2023
@Arshia001 Arshia001 added this to the v4.3 milestone Oct 24, 2023
@Arshia001
Copy link
Member

@yagehu never mind, I just ran it and it does indeed mount the cwd.

@Arshia001 Arshia001 added bug Something isn't working priority-high High priority issue needs-tests and removed priority-medium Medium priority issue ❔ needs more context Needs more context and information from the issue poster. ❓ question I've a question! labels Oct 24, 2023
@Michael-F-Bryan
Copy link
Contributor

I have a feeling the issue comes from the way we set up the preopens here:

let mut builder = if wasmer_wasix::is_wasix_module(module) {
// If we preopen anything from the host then shallow copy it over
let root_fs = RootFileSystemBuilder::new()
.with_tty(Box::new(DeviceFile::new(__WASI_STDIN_FILENO)))
.build();
if !self.mapped_dirs.is_empty() {
let fs_backing: Arc<dyn FileSystem + Send + Sync> =
Arc::new(PassthruFileSystem::new(default_fs_backing()));
for MappedDirectory { host, guest } in self.mapped_dirs.clone() {
let host = if !host.is_absolute() {
Path::new("/").join(host)
} else {
host
};
root_fs.mount(guest.into(), &fs_backing, host)?;
}
}
// Open the root of the new filesystem
builder
.sandbox_fs(root_fs)
.preopen_dir(Path::new("/"))
.unwrap()
.map_dir(".", "/")?
} else {
builder
.fs(default_fs_backing())
.preopen_dirs(self.pre_opened_directories.clone())?
.map_dirs(
self.mapped_dirs
.iter()
.map(|d| (d.guest.clone(), d.host.clone())),
)?
};

@Michael-F-Bryan Michael-F-Bryan changed the title How to enable filesystem sandbox? Running *.wasm files directly doesn't sandbox the filesystem Nov 7, 2023
@theduke
Copy link
Contributor

theduke commented Nov 21, 2023

This was resolved in #4301 .

@theduke theduke closed this as completed Nov 21, 2023
@yagehu
Copy link
Contributor Author

yagehu commented Nov 21, 2023

@theduke Will you publish the security advisory?

@theduke theduke reopened this Dec 1, 2023
@theduke
Copy link
Contributor

theduke commented Dec 1, 2023

@yagehu which version of wasmer are you on, and which operating system?

Because this does not reproduce on wasmer 4.2.3 on Linux.

@Arshia001
Copy link
Member

@theduke you have to compile to wasm32-wasi, the issue does not exist in wasm32-wasmer-wasi.

@Arshia001
Copy link
Member

never mind, it just doesn't happen with cargo wasix run, probably because cargo-wasix changes wasmer's pwd to something else.

@yagehu
Copy link
Contributor Author

yagehu commented Dec 4, 2023

@yagehu which version of wasmer are you on, and which operating system?

Because this does not reproduce on wasmer 4.2.3 on Linux.

This reproduces with v4.2.3 Wasmer on Fedora 39 Linux. It does NOT reproduce with v4.2.4 Wasmer, so I'm assuming your commit fixed it. I'm just wondering if you will publish the security advisory since it impacts people running plain WASI (non WASIX) workloads.

@Arshia001
Copy link
Member

@yagehu we ran into a bit of a hiccup with publishing the crates. That's been resolved and the advisory will be published soon. Tagging @syrusakbary

@Arshia001
Copy link
Member

@yagehu the advisory is now published. I will close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working 📦 lib-vfs About wasmer-vfs needs-tests priority-high High priority issue
Projects
None yet
Development

No branches or pull requests

4 participants