Skip to content

Validate symlink targets in archiveReader and mask high mode bits - #814

Closed
katiewasnothere wants to merge 1 commit into
apple:mainfrom
katiewasnothere:archive_reader_fixes
Closed

Validate symlink targets in archiveReader and mask high mode bits#814
katiewasnothere wants to merge 1 commit into
apple:mainfrom
katiewasnothere:archive_reader_fixes

Conversation

@katiewasnothere

Copy link
Copy Markdown
Contributor

This PR does two things:

  • Prevents target paths for symlinks from linking outside of the extraction root of the archive. Tar entries that attempt to link outside of the extraction root are added to the list of rejected paths returned from extractContents.
  • Masks the high mode bits when setting file attributes for consistency with mode at file creation

Signed-off-by: Kathryn Baldauf <k_baldauf@apple.com>
@iabdullah215

Copy link
Copy Markdown

lexicallyNormalized() crashes in debug builds on some traversal targets (swift-system 1.7.5 assertion). Release is fine, so not an escape, just a debug crash on malicious symlinks.
Repro:

import SystemPackage
_ = FilePath("x/../../../../y").lexicallyNormalized()  // FilePathParsing.swift:231: Assertion failed

Consider checking escape by walking components with a depth counter (reject if it goes negative) instead of normalizing.

@katiewasnothere

Copy link
Copy Markdown
Contributor Author

Closing. We've decided we do not want to limit the symlink targets here. We already use O_NOFOLLOW in the archive code when opening files.

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