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

Should not be able to path_open absolute path #269

Closed
yagehu opened this issue Jun 11, 2024 · 1 comment · Fixed by #270
Closed

Should not be able to path_open absolute path #269

yagehu opened this issue Jun 11, 2024 · 1 comment · Fixed by #270

Comments

@yagehu
Copy link
Contributor

yagehu commented Jun 11, 2024

path_open an absolute path should error. This behavior is consistent amongst Wasmtime, WasmEdge, Wazero, WAMR. Fixing this does not break clients using wasi-libc as it already strips the prefix by matching the path with the appropriate preopened directory.

WASI folks have also stated absolute path should not work at the raw WASI level. WebAssembly/WASI#374

yagehu added a commit to yagehu/uvwasi that referenced this issue Jun 11, 2024
This commit fixes a `path_open` behavior that allows opening absolute
paths. Although the path normalization correctly resolves the path and
enforces the sandbox, it's still a good idea to converge with other
runtimes here.

fixes nodejs#269

Signed-off-by: Yage Hu <me@huyage.dev>
@cjihrig
Copy link
Collaborator

cjihrig commented Jun 11, 2024

@yagehu it would be fantastic if you could implement the openat() algorithm used by WASI for sandboxing. There is some discussion about it in libuv/libuv#4167. You could implement it directly in uvwasi if you don't want to send a PR to libuv. If that were implemented, I believe we could revert 1da5f32

mhdawson pushed a commit that referenced this issue Jun 14, 2024
This commit fixes a `path_open` behavior that allows opening absolute
paths. Although the path normalization correctly resolves the path and
enforces the sandbox, it's still a good idea to converge with other
runtimes here.

fixes #269

Signed-off-by: Yage Hu <me@huyage.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants