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

doc: document dangerous symlink behavior #49154

Merged
merged 1 commit into from
Sep 29, 2023

Conversation

tniessen
Copy link
Member

@tniessen tniessen commented Aug 13, 2023

Much earlier, a design decision was made that the permission model should not prevent following symbolic links to presumably inaccessible locations. Recently, after some back and forth, it had been decided that it is indeed a vulnerability that symbolic links, which currently point to an accessible location, can potentially be re-targeted to point to a presumably inaccessible location. Nevertheless, months later, no solution has been found and the issue is deemed unfixable in the context of the current permission model implementation, so it was decided to disclose the vulnerability peculiarity and to shift responsibiliy onto users who are now responsible for ensuring that no potentially dangerous symlinks exist in any directories that they grant access to.

I believe that this design issue might be surprising and that it comes with significant security implications for users, so it should be documented.

Original vulnerability report: https://hackerone.com/reports/1961655

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/security-wg

@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Aug 13, 2023
@RafaelGSS RafaelGSS self-requested a review August 14, 2023 00:35
doc/api/permissions.md Show resolved Hide resolved
doc/api/permissions.md Outdated Show resolved Hide resolved
@RafaelGSS
Copy link
Member

Hey @tniessen, do you know some help on this? Feel free to ping me.

@RafaelGSS RafaelGSS added the permission Issues and PRs related to the Permission Model label Sep 7, 2023
@tniessen
Copy link
Member Author

I think it's technically correct as it is, but I'll rephrase it.

Much earlier, a design decision was made that the permission model
should not prevent following symbolic links to presumably inaccessible
locations. Recently, after some back and forth, it had been decided that
it is indeed a vulnerability that symbolic links, which currently point
to an accessible location, can potentially be re-targeted to point to a
presumably inaccessible location. Nevertheless, months later, no
solution has been found and the issue is deemed unfixable in the context
of the current permission model implementation, so it was decided to
disclose the vulnerability and to shift responsibiliy onto users who are
now responsible for ensuring that no potentially dangerous symlinks
exist in any directories that they grant access to.

I believe that this design issue might be surprising and that it comes
with significant security implications for users, so it should be
documented.

Original vulnerability report: https://hackerone.com/reports/1961655
@mcollina
Copy link
Member

@tniessen the content of that report was closed as "informative", based on what it was discussed at length this is a limitation of the current system, and not a vulnerability that is solvable without some significant redesign, and better handled in public vs private doors. Therefore, no, that's not really a vulnerability per our processes. Could you amend the PR description?

@tniessen
Copy link
Member Author

@mcollina I have updated the PR description. Let me know if anything in there is untrue.

For what it's worth, there was agreement that this is a vulnerability. The reason for not following our usual vulnerability procedures was not a reassessment of the issue, but the inability to fix it even after months.

@tniessen tniessen added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Sep 24, 2023
@tniessen tniessen added the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 29, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 29, 2023
@nodejs-github-bot nodejs-github-bot merged commit 1dc0667 into nodejs:main Sep 29, 2023
20 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 1dc0667

alexfernandez pushed a commit to alexfernandez/node that referenced this pull request Nov 1, 2023
Much earlier, a design decision was made that the permission model
should not prevent following symbolic links to presumably inaccessible
locations. Recently, after some back and forth, it had been decided that
it is indeed a vulnerability that symbolic links, which currently point
to an accessible location, can potentially be re-targeted to point to a
presumably inaccessible location. Nevertheless, months later, no
solution has been found and the issue is deemed unfixable in the context
of the current permission model implementation, so it was decided to
disclose the vulnerability and to shift responsibiliy onto users who are
now responsible for ensuring that no potentially dangerous symlinks
exist in any directories that they grant access to.

I believe that this design issue might be surprising and that it comes
with significant security implications for users, so it should be
documented.

Original vulnerability report: https://hackerone.com/reports/1961655

PR-URL: nodejs#49154
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
targos pushed a commit that referenced this pull request Nov 11, 2023
Much earlier, a design decision was made that the permission model
should not prevent following symbolic links to presumably inaccessible
locations. Recently, after some back and forth, it had been decided that
it is indeed a vulnerability that symbolic links, which currently point
to an accessible location, can potentially be re-targeted to point to a
presumably inaccessible location. Nevertheless, months later, no
solution has been found and the issue is deemed unfixable in the context
of the current permission model implementation, so it was decided to
disclose the vulnerability and to shift responsibiliy onto users who are
now responsible for ensuring that no potentially dangerous symlinks
exist in any directories that they grant access to.

I believe that this design issue might be surprising and that it comes
with significant security implications for users, so it should be
documented.

Original vulnerability report: https://hackerone.com/reports/1961655

PR-URL: #49154
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
debadree25 pushed a commit to debadree25/node that referenced this pull request Apr 15, 2024
Much earlier, a design decision was made that the permission model
should not prevent following symbolic links to presumably inaccessible
locations. Recently, after some back and forth, it had been decided that
it is indeed a vulnerability that symbolic links, which currently point
to an accessible location, can potentially be re-targeted to point to a
presumably inaccessible location. Nevertheless, months later, no
solution has been found and the issue is deemed unfixable in the context
of the current permission model implementation, so it was decided to
disclose the vulnerability and to shift responsibiliy onto users who are
now responsible for ensuring that no potentially dangerous symlinks
exist in any directories that they grant access to.

I believe that this design issue might be surprising and that it comes
with significant security implications for users, so it should be
documented.

Original vulnerability report: https://hackerone.com/reports/1961655

PR-URL: nodejs#49154
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. doc Issues and PRs related to the documentations. permission Issues and PRs related to the Permission Model
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants