unblob icon indicating copy to clipboard operation
unblob copied to clipboard

Sanitize symlink target

Open e3krisztian opened this issue 1 year ago • 2 comments

Split off of #763 . There are still problems to solve here, see https://github.com/onekey-sec/unblob/commit/954c1cd5a06bcdb52048fc23da44661c73c94f31#commitcomment-138623089 but tests should run with the exception of 2 failures.

https://github.com/onekey-sec/unblob/commit/954c1cd5a06bcdb52048fc23da44661c73c94f31 rewrites the logic to sanitize symlinks to be relative and kept within the extraction directory. This is done using the os module instead of Pathlib as Pathlib.resolve would fail if a symlink target was missing (which doesn't prevent us from safely converting it to a relative link). With this change I no longer see false positives around MaliciousSymlinks, instead symlinks are created safely within the extraction directory. If a relative symlink originally tried accessing a directory above its own root (i.e., ./bin/sh -> ../../../../../bin/bash), we update the link so it remains within the extraction directory.

e3krisztian avatar Feb 14 '24 15:02 e3krisztian

I believe there is still a bug in b11fe469a8716e7c00e70f4daa99e291a0277422: when extracting in a (host) directory within /tmp/, a symlink of /var pointing to /tmp/var will be sanitize to point to var creating a symlink loop. This seems to be caused by the use of os.path.commonpath finding a similarity between the extraction directory and the absolute symlink target.

AndrewFasano avatar Feb 21 '24 21:02 AndrewFasano

@qkaiser I wanted to make this PR just to have a place to discuss. It was extracted from a larger PR, and wanted to see CI test results, which our code checks (ruff) prevented, to push through the commit I have made some hacky fixes to be thrown away in the final version.

This definitely needs a rewrite, so should have made it draft initially.

I do not plan working on this soon, especially as the tar fix covering these problems are merged.

e3krisztian avatar Feb 22 '24 10:02 e3krisztian