Skip to content

contain assay file reads within the isa-tab directory#2873

Open
digi-scrypt wants to merge 1 commit into
apache:mainfrom
digi-scrypt:isatab-assay-path-traversal
Open

contain assay file reads within the isa-tab directory#2873
digi-scrypt wants to merge 1 commit into
apache:mainfrom
digi-scrypt:isatab-assay-path-traversal

Conversation

@digi-scrypt

Copy link
Copy Markdown

Assay reads in the ISA-Tab parser trust a file name straight from the parsed document:

  • parseAssay loops over every "Study Assay File Name" value pulled out of the investigation file and joins it onto the location dir with no containment
  • the value is attacker controlled, so "../../../etc/passwd" makes the parser open and stream a file from outside the archive folder into the output

What happens with a relative name that climbs out? Before this it just followed it. resolveWithinLocation now canonicalizes the candidate and rejects anything that doesn't sit under location (also closes the symlink variant). Test crafts an investigation pointing an assay at a sibling secret file and checks it no longer leaks.

@tballison

Copy link
Copy Markdown
Contributor

Thank you for opening this. For some reason, I thought this was opened and then deleted. I fixed this two weeks ago, I think. Let me know if current 3.x or main require further work.

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.

2 participants