tarfile and directory traversal vulnerability
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 hi, once upon a time there was a known vulnerability in tar (CVE-2001-1267, [1]), and while tar is now long fixed, python's tarfile module is affected too. The vulnerability goes basically like this: If you tar a file named "../../../../../etc/passwd" and then make the admin untar it, /etc/passwd gets overwritten. Another variety of this bug is a symlink one: if tar contains files like: ./aaaa-directory -> /etc ./aaaa-directory/passwd then the "aaaa-directory" symlink would be created first and /etc/passwd will be overwritten once again. I was wondering how to fix it. The symlink problem obviously applies only to extractall() method and is easily fixed by delaying external (or possibly all) symlink creation, similar to how directory attributes are delayed now. I've attached a draft of the patch, if you like it, i'll polish it. The traversal problem is harder, and it applies to extract() method as well. For extractall() alone, i would use something like: if tarinfo.name.startswith('../'): self.extract(tarinfo, path) else: warnings.warn("non-local file skipped: %s" % tarinfo.name, RuntimeWarning, stacklevel=1) For extract(), i am not sure. Maybe it should throw exception when it encounters such file, and have a special option to extract such files anyway. Or maybe it should be left alone altogether. Any suggestions are welcome. regards jan matejek [1] http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2001-1267 -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.4-svn0 (GNU/Linux) Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org iD8DBQFGzxcpjBrWA+AvBr8RAlduAKCk0iiSoBF+wA9xgXmDlpWsECZ7KgCfQORg lZ85inT1FGwhGqBfxJvCGGU= =TiWx -----END PGP SIGNATURE-----
I must admit I fail to see the bug. If root untars a file, and that tar file contains an instruction to overwrite /etc/passwd, why is an error to execute that instruction? Shouldn't root just be more careful when untaring files?
Ok. You seem to be claiming that the tarfile is incorrect in some sense. Can you please point to some spec that says this is an incorrect tarfile? In any case, if you fix what you consider broken, you should do it exactly the same way as GNU tar does it (assuming you consider GNU tar fixed). Regards, Martin
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Martin v. Löwis wrote:
GNU tar is not supposed to place files outside its working directory, unless explicitly specified otherwise. So this is considered a security vulnerability. AFAIK there is no specified behavior and other tars might act differently. But i think GNU tar behaves correctly in this regard. Furthermore, extract() and extractall() documentation says "Extract (...) from the archive to the *current working directory* or directory [path]." So current behavior is actually inconsistent with the documentation.
No, the tar file itself is correct, according to POSIX. You can put anything into a tar. Point is, you should be able to untar any file 'safely'.
I can do that. I would propose an optional parameter for extract() and extractall(), absolutePaths, defaulting to False. When encountering a non-local file, it would strip the leading slash or the path up to the last '../' sequence (that is what GNU tar does) and extract the file locally. Setting absolutePaths to True would restore current behavior (no checks). regards, jan matejek -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.4-svn0 (GNU/Linux) Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org iD8DBQFG0wtkjBrWA+AvBr8RAmmnAKCtpYYoFZYaNwba2WW11NtRuCyqhwCePkFw 9M2pKHtu0O62fAYfb8NTm3A= =yfVK -----END PGP SIGNATURE-----
So that's a vulnerability in GNU tar, sure - it does something that it is not supposed to do. But why is there also a vulnerability in tarfile.py? It does very well what it is supposed to do.
AFAIK there is no specified behavior and other tars might act differently.
I think you are mistaken here. POSIX specifies something (although I'm uncertain what precisely) for pax(1); this ended the tar wars.
Ok. However, what does it mean to create a file with an absolute path in the current directory? Also, it's fairly easy to see what creating "../foo" should do when done in the current directory: create a sibling of the current directory.
I see, you are asking for an option. If people want to have this option, it should be added. Then, of course, the question is what default it should take. Regards, Martin
On Fri, Aug 24, 2007 at 07:36:41PM +0200, Jan Matejek wrote:
tarfile currently contains no sanity checks at all. The easiest way to attack /etc/passwd would be to give tarfile a tar created with `tar -cPf foo.tar /etc/passwd'.
Suppose we have: foo -> /etc foo/passwd If creation of the foo symlink is delayed, foo/passwd will be extracted in a directory foo which will be created implicitly. If we create the foo symlink afterwards it will fail because foo already exists. The best way would be to completely ignore members and link targets that are absolute or outside the archive's scope.
Yes, I think that is the right way to do it. -- Lars Gustäbel lars@gustaebel.de A chicken is an egg's way of producing more eggs. (Anonymous)
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Lars Gustäbel wrote:
GNU tar doesn't descend into symlinked directories when extracting, such archive fails anyway: # tar xvf foo.tar foo foo/passwd tar: foo/passwd: Cannot open: Not a directory tar: Error exit delayed from previous errors I think that is the simplest solution, but i'm not sure how to best implement that in extractall(). -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.4-svn0 (GNU/Linux) Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org iD8DBQFG0wyUjBrWA+AvBr8RAjkJAKCJS+hkV1HYL9egOsyeTE5vj44r5ACeNmt7 HquYw+ON+5qVNoC778OtQRE= =9Kx/ -----END PGP SIGNATURE-----
On Mon, Aug 27, 2007 at 07:40:36PM +0200, Jan Matejek wrote:
GNU tar creates a placeholder file for every hard or symbolic link during the extract process and in a second step replaces them with links. I don't think that this is a good choice for a library. The problem is that it leads to delayed and (from the user's POV) unrelated errors. I prefer the solution that archive members with pathnames that either start with a "/" or a "../" raise an exception by default and can be extracted only by direct request. I am currently working on a patch. Should we move this discussion over to the bugtracker? -- Lars Gustäbel lars@gustaebel.de Linux is like a wigwam - no Gates, no Windows, Apache inside.
participants (3)
-
"Martin v. Löwis"
-
Jan Matejek
-
Lars Gustäbel