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

Allow read_link to read absolute paths #354

Closed
wants to merge 1 commit into from

Conversation

asomers
Copy link
Contributor

@asomers asomers commented May 14, 2024

Previously it would return an error if the link's destination were absolute. But there were three problems with this:

  • It wouldn't return an error if the link's destination were a relative path that uses ../ to point outside of the sandbox. That's inconsistent.
  • Sometimes a symlink with an absolute path doesn't escape the sandbox. For example /sandbox/link -> /sandbox/target. But read_link wouldn't allow that.
  • More importantly, sometimes it's important to read a link that points outside of the sandbox. For example, a backup application could copy all of the files from the root directory of one computer to a subdirectory of another. Symlinks would be broken, but would work again after a restore operation. The application that performs the restore would need to be able to read link targets, even if they point outside of the root. Or, a jail file system could contain absolute symlinks that resolve correctly for a jailed process, but not for an unjailed one. But it would still sometimes be useful for an unjailed process to read the links.

Fixes #353

@asomers asomers force-pushed the readlinkat branch 2 times, most recently from 04e1cad to 73bc997 Compare May 14, 2024 16:28
Previously it would return an error if the link's destination were
absolute.  But there were three problems with this:

* It wouldn't return an error if the link's destination were a relative
  path that uses ../ to point outside of the sandbox. That's
  inconsistent.
* Sometimes a symlink with an absolute path doesn't escape the sandbox.
  For example /sandbox/link -> /sandbox/target.  But read_link wouldn't
  allow that.
* More importantly, sometimes it's important to read a link that points
  outside of the sandbox. For example, a backup application could copy
  all of the files from the root directory of one computer to a
  subdirectory of another. Symlinks would be broken, but would work
  again after a restore operation. The application that performs the
  restore would need to be able to read link targets, even if they point
  outside of the root. Or, a jail file system could contain absolute
  symlinks that resolve correctly for a jailed process, but not for an
  unjailed one. But it would still sometimes be useful for an unjailed
  process to read the links.

Fixes bytecodealliance#353
asomers added a commit to asomers/libunftp that referenced this pull request May 14, 2024
asomers added a commit to asomers/cap-std that referenced this pull request May 15, 2024
These allow creating and readling symlinks whose target may be an
absolute path.  There is no security risk because security is as always
provided by the OS; the OS's openat implementation will refuse to follow
a symlink whose target is an absolute path.

Fixes bytecodealliance#354
asomers added a commit to asomers/cap-std that referenced this pull request May 15, 2024
These allow creating and readling symlinks whose target may be an
absolute path.  There is no security risk because security is as always
provided by the OS; the OS's openat implementation will refuse to follow
a symlink whose target is an absolute path.

Fixes bytecodealliance#354
@asomers
Copy link
Contributor Author

asomers commented May 15, 2024

Superseded by #356

@asomers asomers closed this May 15, 2024
sunfishcode pushed a commit that referenced this pull request May 16, 2024
These allow creating and readling symlinks whose target may be an
absolute path.  There is no security risk because security is as always
provided by the OS; the OS's openat implementation will refuse to follow
a symlink whose target is an absolute path.

Fixes #354
asomers added a commit to asomers/libunftp that referenced this pull request May 28, 2024
asomers added a commit to asomers/libunftp that referenced this pull request May 28, 2024
asomers added a commit to asomers/libunftp that referenced this pull request May 28, 2024
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.

read_link should be allow absolute paths
1 participant