-
Notifications
You must be signed in to change notification settings - Fork 44
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
cabextract: Writing into symlinks #42
Comments
Thank you, this is a good issue to bring up. There are two things to consider here; first is about following symlinks or not, the second is overwriting or not. cabextract currently has no options for controlling overwrite behaviour; it would great to add some. I'm not sure whether following symlinks is bad or not. As cabinet files cannot contain symlinks, it's not a security issue but a user choice. Only the user can introduce symlinks to cabextract's extraction path. cabextract follows default unix semantics; overwriting a symlinked file writes to its destination. This might even be desirable to the user. Looking further into tar and zip:
tar's approach is simpler but requires a system that conforms to POSIX.2008, zip's approach is more portable. What should cabextract do? Should it maintain its existing behaviour (is someone relying on its current overwrite behaviour?), or should it move towards being more like unzip/tar behaviour? My thoughts are to be more like unzip/tar, but the user need only add one flag to get the old behaviour back:
The option help would be:
What do you think? Thanks for raising the issue; if you'd like to, you can continue to develop this feature and raise a pull request, but if you don't want to, I'm also OK taking this as a feature request and developing it myself. |
This seems sane to me. To summarize:
|
Your summary is correct. For For My thoughts would be to add a function call before extracting, e.g.
can_write() would have logic like this:
|
Is there any reason for not merging this #43 PR? This issue breaks installation of any CAB-based package in winetricks/protontricks, ie. most packages. |
The reason it's not merged is that it's incomplete; there is nothing wrong with what's written so far, but it only goes so far as to add a test and the command-line parsing. I'll take a look into finishing it |
Thank you, much appreciated. The newer versions of Proton create only symlinks in SYSTEM32/SYSWOW64, which breaks attempts to fix compatibility issues. |
Any updates on getting this flushed out and finished? |
I've implemented the remaining parts, and would appreciate testing of the new functionality, if you have time. |
Hi @kyz, I see this has made it into a release. A couple questions:
|
Hi @austin987
Also, to clarify further: if you run
|
Perfect, thanks! |
The problem
Let's say we have a CAB file that contains
hello.dll
and a symlink with the same name to some other file in the current directory.With this when you extract the CAB the symlink is followed and it's target file is overwritten.
Why is it problem
AFAIK when writing files, following file symlinks is generally unsafe (?), at least within context of archive extraction (again, afaik)
EDIT: Tried what other archive tools do
Info-ZIP
) asks what to do when it finds a symlink (with same name as to-be extracted file)tar -xf
ignores the symlink (eg. unlinks and writes extracted file)I've made a super simple patch in my fork https://github.com/wereii/libmspack/commits/master that adds build option for opting into symlink checks before a file is opened for writing (and unlinking/removing possible link before the
fopen
happens, so the file is written as is and not into the symlinked file, nothing more).Though the patch is just a simple thing that works for me and
Either way this behavior requires some more though so here we are.
The text was updated successfully, but these errors were encountered: