Skip to content

fix: validate symlink targets to prevent prefix escape#2143

Merged
baszalmstra merged 4 commits intoconda:mainfrom
Premkumar-2004:issue-symlink
Mar 3, 2026
Merged

fix: validate symlink targets to prevent prefix escape#2143
baszalmstra merged 4 commits intoconda:mainfrom
Premkumar-2004:issue-symlink

Conversation

@Premkumar-2004
Copy link
Copy Markdown
Contributor

@Premkumar-2004 Premkumar-2004 commented Feb 27, 2026

Description

Symlink targets weren't being validated anywhere. A malicious package could include a symlink like lib/foo ../../../../.bashrc and it'd get created as-is in the prefix.

Disabled allow_external_symlinks in the tar ArchiveBuilder,Added target validation in symlink_to_destination before linking,Added target validation in validate_package_soft_link_entry

How Has This Been Tested?

Tested locally symlinks escaping the prefix are now rejected.

AI Disclosure

  • This PR contains AI-generated content.
    • I have tested any AI-generated content in my PR.
    • I take responsibility for any AI-generated content in my PR.

Tools: claude

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added sufficient tests to cover my changes.

Copy link
Copy Markdown
Collaborator

@baszalmstra baszalmstra left a comment

Choose a reason for hiding this comment

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

thanks! Left one comment, otherwise looks good!

Comment thread crates/rattler_cache/src/validation.rs Outdated
Copy link
Copy Markdown
Collaborator

@baszalmstra baszalmstra left a comment

Choose a reason for hiding this comment

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

Do you think you could also add a test to verify that paths now cannot escape the package?

@Premkumar-2004
Copy link
Copy Markdown
Contributor Author

Premkumar-2004 commented Feb 27, 2026

i'll add a test case that confirms symlink escaping the prefix gets rejected .

@Premkumar-2004
Copy link
Copy Markdown
Contributor Author

i have one more pr which i raised a week before could you merge that #2090

@wolfv
Copy link
Copy Markdown
Contributor

wolfv commented Feb 27, 2026

I am not sure this will be the right thing to do. We migth have to discsus this wiht teh conda community. There migth be packages that legitimately ship symlinks to outside the env? E.g. for drivers?

@Premkumar-2004
Copy link
Copy Markdown
Contributor Author

I hadn't considered driver packages. We could log a warning instead of rejecting, so nothing breaks but it's still visible. Or make it configurable so stricter setups can opt into hard failing. i can adjust either way, let me know what you'd prefer.

@wolfv
Copy link
Copy Markdown
Contributor

wolfv commented Feb 27, 2026

Yeah we do have a setting that allows / disallows running of post-link scripts. Something similar perhaps?

@Premkumar-2004
Copy link
Copy Markdown
Contributor Author

Yeah I can add an allow_external_symlinks flag to the install options, similar to how post-link scripts are handled.

@Premkumar-2004
Copy link
Copy Markdown
Contributor Author

Premkumar-2004 commented Mar 2, 2026

@baszalmstra can you please review the pr now i have updated it and can you please re-run the testcases

@baszalmstra baszalmstra enabled auto-merge (squash) March 3, 2026 10:51
@baszalmstra baszalmstra merged commit 26a2c59 into conda:main Mar 3, 2026
29 of 31 checks passed
@github-actions github-actions Bot mentioned this pull request Mar 2, 2026
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