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

FileSystem: Don't follow symbolic links when recursively deleting directories #12052

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

chaoticgd
Copy link
Contributor

@chaoticgd chaoticgd commented Dec 2, 2024

Description of Changes

  • The FileSystem::RecursiveDeleteDirectory function will no longer recurse through symbolic links.
  • To support this change, the FileSystem::IsSymbolicLink and FileSystem::DeleteSymbolicLink functions have been added. The FileSystem::IsRealDirectory function has been removed since it was unused.

Rationale behind Changes

Prevent accidental data loss e.g. when deleting folder memory cards.

Suggested Testing Steps

This needs testing on macOS and especially Windows, since it touches Windows API code and I've only tested it on Linux. Since this is modifying some fairly important code I want multiple core contributors to review it before it's merged.

Here's an example of how you could reproduce the issue on Linux/macOS:

cd path/to/memcards/directory/
pcsx2-qt # create a folder card called "testcard" from the gui
mkdir important_dir
nano important_dir/important_file.txt # add only copy of private key for 1000000 bitcoin reserve
ln -s $(realpath important_dir) testcard.ps2/link # it's important to use an absolute path here
pcsx2-qt # try to delete "testcard" from the gui
ls important_dir/important_file.txt # oh no!

Be careful that you don't e.g. symlink your home directory by accident while testing this, as you could lose all your data.

@chaoticgd chaoticgd force-pushed the dont_follow_symlinks branch 4 times, most recently from 2809506 to ffbea15 Compare December 2, 2024 17:52
@chaoticgd
Copy link
Contributor Author

Updated the test so it always creates a fresh directory.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant