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

fix(std/wasi): disallow path_open outside of pre-opened dirfd #8078

Conversation

caspervonb
Copy link
Contributor

@caspervonb caspervonb commented Oct 22, 2020

Disallow path_open to open paths outside of the dirfd that is being accessed.

@caspervonb caspervonb changed the title [WIP] fix(std/wasi): disallow path_open outside of pre-opened dirfd fix(std/wasi): disallow path_open outside of pre-opened dirfd Oct 22, 2020
@caspervonb caspervonb marked this pull request as ready for review October 22, 2020 23:17
@caspervonb caspervonb marked this pull request as draft October 22, 2020 23:31
@caspervonb caspervonb marked this pull request as ready for review October 22, 2020 23:56
@caspervonb
Copy link
Contributor Author

Think I might have stumbled upon a bug with symlinks and/or std/fs/copy.
Unix works fine, but on Windows symlink_to_parent_directory (a relative symlink with ..) doesn't appear to be a symlink at all.

Don't have a windows machine to work on this with so this is very difficult to debug.

@lucacasonato
Copy link
Member

Unix works fine, but on Windows symlink_to_parent_directory (a relative symlink with ..) doesn't appear to be a symlink at all.

cc @piscisaureus

@caspervonb
Copy link
Contributor Author

caspervonb commented Oct 23, 2020

I'm appending this new test to the growing list of ignored windows tests for the time being until I get around to provisioning a Windows based server to fix them all.

Windows behaviour isn't any worse than it was before this commit, the new tests just make it more obvious that some symlink cases are probably broken on Windows, most likely the test runner itself is at fault.

Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bartlomieju bartlomieju merged commit 2a83b22 into denoland:master Oct 24, 2020
caspervonb added a commit to caspervonb/deno_std that referenced this pull request Jan 21, 2021
caspervonb added a commit to caspervonb/deno_std that referenced this pull request Jan 24, 2021
caspervonb added a commit to caspervonb/deno_std that referenced this pull request Jan 24, 2021
caspervonb added a commit to caspervonb/deno_std that referenced this pull request Jan 24, 2021
caspervonb added a commit to caspervonb/deno_std that referenced this pull request Jan 31, 2021
caspervonb added a commit to caspervonb/deno_std that referenced this pull request Jan 31, 2021
caspervonb added a commit to caspervonb/deno_std that referenced this pull request Jan 31, 2021
caspervonb added a commit to caspervonb/deno_std that referenced this pull request Jan 31, 2021
caspervonb added a commit to caspervonb/deno_std that referenced this pull request Feb 1, 2021
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 this pull request may close these issues.

3 participants