Author shanxS
Recipients Arfrever, Daniel.Garcia, Philippe.Godbout, benjamin.peterson, christian.heimes, edulix, georg.brandl, jcea, jwilk, lars.gustaebel, martin.panter, ned.deily, r.david.murray, serhiy.storchaka, shanxS, taleinat, vstinner
Date 2018-09-13.04:03:14
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1536811395.97.0.956365154283.issue21109@psf.upfronthosting.co.za>
In-reply-to
Content
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"
History
Date User Action Args
2018-09-13 04:03:17shanxSsetrecipients: + shanxS, georg.brandl, jcea, lars.gustaebel, vstinner, taleinat, christian.heimes, benjamin.peterson, jwilk, ned.deily, Arfrever, r.david.murray, martin.panter, serhiy.storchaka, edulix, Daniel.Garcia, Philippe.Godbout
2018-09-13 04:03:15shanxSsetmessageid: <1536811395.97.0.956365154283.issue21109@psf.upfronthosting.co.za>
2018-09-13 04:03:15shanxSlinkissue21109 messages
2018-09-13 04:03:15shanxScreate