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

Interactive File Overwrite Prompt can be Circumvented by Sending ZIP file #594

Closed
mgerstner opened this issue Sep 8, 2023 · 4 comments · Fixed by #698
Closed

Interactive File Overwrite Prompt can be Circumvented by Sending ZIP file #594

mgerstner opened this issue Sep 8, 2023 · 4 comments · Fixed by #698
Assignees
Labels

Comments

@mgerstner
Copy link

As mentioned in issue #593 there is a check on the receiver side of Croc whether an incoming file path will overwrite an existing file, and Croc will ask the user interactively whether it should be overwritten, if it already exists. This check works reasonably well, even if symlinks are involved. I did not look that deep into this though - due to the following finding I stopped spending time on finding further possible ways around the restriction. There might still be attack surface to be found in the parallelism of the chunk transfer (exploiting race conditions) or by aptly crafting the file metadata.

There is a loophole in conjunction with the transparent ZIP transfer option though (croc send --zip). The sender alone decides if something will be zipped or not (somewhat confusingly flagged by the FileInfo.TempFile flag).

The receiver will take whatever data has been transferred and will try to unzip it. This way even the overwrite check can be overcome, by placing creative paths into the ZIP archive. Even relative paths like ../../.ssh/authorized_keys can be placed in the ZIP archive. The unzip operation will silently overwrite existing files.

When combined with issue #593 then a potential attacker has a lot of freedom to attempt to trick the receiving party into harming its system.

If an isolation technique as outlined in issue #593 is implemented, then the consequences of overwriting files should be less problematic - although it could still be surprising if previously downloaded files are overwritten with something else. As an additional protection measure only the receiver should decide whether ZIP files are handled, or not. Better controlling the unzip process to prevent overwrites would also be helpful. The safest approach would be to use a pristine empty directory for each new file transfer where nothing can be overwritten in the first place.

@schollz
Copy link
Owner

schollz commented Sep 20, 2023

Thanks, this would be a great PR to have

@mgerstner
Copy link
Author

Mitre assigned CVE-2023-43616 to track this issue.

@aspann
Copy link

aspann commented Feb 3, 2024

Shouldn't CVE-2023-43616 cover 9.6.6 as well?

@schollz
Copy link
Owner

schollz commented May 20, 2024

continue discussion on #698

@schollz schollz closed this as completed May 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants