dotaxis / 7thDeck

Installer for 7th Heaven on Steam Deck
84 stars 5 forks source link

Running `rm -rf "${WINEPATH%/pfx}"/* ` is very risky #32

Closed chpoit closed 3 months ago

chpoit commented 3 months ago

Hey, you should add some safety checks around the rm -rf "${WINEPATH%/pfx}"/* in your install script. If your Z drive is mounted inside wine, you're going to nuke the entirety of people's drives.

Luckily, I realized what it was doing and I "only" lost my kde config and the steam games I had installed, and since my root folder is on a separate partition, I don't need to do a full reinstall, but still, you should at least add a "do you want to delete the wine directory" prompt with the risks, just in case.

I did run this on my desktop, but there's always the risk of someone having "unlocked" their steamdeck filesystem and having it nuked.

dotaxis commented 3 months ago

I'm not sure how this would possibly nuke your whole filesystem? WINEPATH is picked up programmatically by reading Steam's libraryfolders.vdf. It points to FF7's proton prefix. Deleting the contents of this folder should not be doing anything to the root of your filesystem under any circumstances.

If I'm somehow wrong about this I'm very sorry, but I really don't think my script is responsible for this.

I will ask some peers to review this issue and perhaps weigh in on it themselves.

chpoit commented 3 months ago

It does so through the "dosdevices" folder Screenshot_20240331_085800 The Z: drive is a symlink to your linux (/) drive/path. So when rm -rf runs it goes through the symlink and starts nuking your files

To be honest, I have no idea what causes the drive to be mounted or not. I know of this issue from another time when I deleted a pfx from dolphin and ended up having to do a full reinstall, but it's been a year or two. I have deleted a few pfx's since and never ran into this issue until yesterday where I saw it log failures to delete some files.

dotaxis commented 3 months ago

Z: is always mounted in a proton prefix. 7thDeck depends on this to function. Running rm -rf doesn't follow symlinks, it removes them.

Unless you are on some distro with changed rm behaviour, what has happened to you is only possible if you modified the script somehow to bypass the check for FF7.

chpoit commented 3 months ago

I'm running arch, I shouldn't have done anything to rm, so I have no idea what kind of freak event causes deletion to go inside a symlink, feel free to close this if you think it's not worth looking into further.

According to this a comment on this reply https://superuser.com/a/382316 having the /* goes inside the symlink and deletes files, except it didn't have that behavior on my steamdeck, so eh.

dotaxis commented 3 months ago

It would only remove the contents of the symlinked directory if the command pointed directly to the symlink's contents, for example rm -rf $WINEPATH/dosdevices/z:/* would be very damaging but rm -rf $WINEPATH/dosdevices/* and rm -rf $WINEPATH/dosdevices/z: are both perfectly safe.

Like I said, the only way this script tried to delete your root is if you modified the script to skip FF7 detection without manually setting WINEPATH.

I appreciate the report because that line did have potential for disaster in the event of future changes to the script. Closing because https://github.com/dotaxis/7thDeck/pull/33 accounts for this now.