shashank <shanx.shash...@gmail.com> added the comment:
1. I have done some changes to Lar's patch to address class of bugs which Jakub found. Attached patch safetarfile-2.diff Patch is for code only and is work in progress. 2. However, there maybe several edge cases which have not been covered. Going by types of bugs we want to catch: * Don't allow creating files whose absolute path is not under the destination. * Don't allow creating links (hard or soft) which link to a path outside of the destination. * Don't create device nodes. I suspect there may be more so which haven't been mentioned yet, one of which I have listed below. 3. Now, on to patch, Safetar now tries to keep a list of symlinks it has seen so far and tries to figure effective path of current name which may use symlinks. This approach does address bugs found in Jakub's comment, details below, but when symlink's link has a symlink in it, there are cases which this impl. let slip. For example: # tar -tvf dirsym_rouge.tar drwxr-xr-x root/root 0 2018-09-12 19:03 dirsym/ lrwxrwxrwx root/root 0 2018-09-12 18:39 dirsym/sym -> . lrwxrwxrwx root/root 0 2018-09-12 19:02 dirsym/symsym3 -> ../dirsym/sym/../.. This escapes the check since, given name "../dirsym/sym/../.." translates to "..", ideally this should have given relative link warning. Above symlink is valid. But for: # tar -tvf dirsym.tar drwxr-xr-x root/root 0 2018-09-12 18:44 dirsym/ lrwxrwxrwx root/root 0 2018-09-12 18:44 dirsym/sym -> . lrwxrwxrwx root/root 0 2018-09-12 18:44 dirsym/symsym -> dirsym/sym/../.. This fails with warning of relative link name, as expected. given name "dirsym/sym/../.." translates to "../.." However, the symlink points to invalid path which may or maynot be useful. 4. Regarding bugs reported by Jakub, following enumerates the effective name that gets computed by Safetar: absolute1.tar "/tmp/moo" translates to "/tmp/moo" absolute2.tar "//tmp/moo" translates to "//tmp/moo" dirsymlink.tar "tmp" translates to "tmp" "/tmp" translates to "tmp" dirsymlink2a.tar "cur" translates to "cur" "." translates to "." "par" translates to "par" "cur/.." translates to ".." "par/moo" translates to "../moo" dirsymlink2b.tar "cur" translates to "cur" "." translates to "." "cur/par" translates to "par" ".." translates to ".." "par/moo" translates to "../moo" relative0.tar "../moo" translates to "../moo" relative2.tar "tmp/../../moo" translates to "../moo" symlink.tar "moo" translates to "moo" "/tmp/moo" translates to "/tmp/moo" ---------- Added file: https://bugs.python.org/file47800/safetarfile-2.diff _______________________________________ Python tracker <rep...@bugs.python.org> <https://bugs.python.org/issue21109> _______________________________________ _______________________________________________ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com