[issue1044] tarfile insecure pathname extraction

[Tal Einat]

Change by Tal Einat :


--
Removed message: https://bugs.python.org/msg324192

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1044] tarfile insecure pathname extraction

[Tal Einat]

Tal Einat  added the comment:

I suggest marking this as a duplicate of #21109, which is more general and 
includes most of the relevant discussion and patches.

--
nosy: +taleinat

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1044] tarfile insecure pathname extraction

[Matthias Klose]

Changes by Matthias Klose d...@debian.org:


--
nosy: +doko

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1044
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1044] tarfile insecure pathname extraction

[Lars Gustäbel]

Lars Gustäbel added the comment:

After careful consideration and a private discussion with Martin I do no
longer think that we have a security issue here. tarfile.py does nothing
wrong, its behaviour conforms to the pax definition and pathname
resolution guidelines in POSIX. There is no known or possible practical
exploit.

I update the documentation with a warning, that it might be dangerous to
extract archives from untrusted sources. That is the only thing to be
done IMO.

--
type: security - behavior

__
Tracker [EMAIL PROTECTED]
http://bugs.python.org/issue1044
__
___
Python-bugs-list mailing list 
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1044] tarfile insecure pathname extraction

[jan matejek]

jan matejek added the comment:

if that can be considered official stance, it's fine by me. feel free
to close the bug.

__
Tracker [EMAIL PROTECTED]
http://bugs.python.org/issue1044
__
___
Python-bugs-list mailing list 
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1044] tarfile insecure pathname extraction

[Lars Gustäbel]

Lars Gustäbel added the comment:

I updated the documentation, r57764 (trunk) and r57765 (2.5).

--
resolution:  - works for me
status: open - closed

__
Tracker [EMAIL PROTECTED]
http://bugs.python.org/issue1044
__
___
Python-bugs-list mailing list 
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1044] tarfile insecure pathname extraction

[Lars Gustäbel]

New submission from Lars Gustäbel:

tarfile does not check pathnames or linknames on extraction. This can
lead to data loss or attack scenarios when members with absolute
pathnames or pathnames outside of the archive's scope overwrite or
overlay existing files or directories.

Example for a symlink attack against /etc/passwd:

foo - /etc
foo/passwd

--
assignee: lars.gustaebel
components: Library (Lib)
files: insecure_pathnames.diff
keywords: patch
messages: 55361
nosy: lars.gustaebel, matejcik
priority: normal
severity: normal
status: open
title: tarfile insecure pathname extraction
type: security
versions: Python 2.6

__
Tracker [EMAIL PROTECTED]
http://bugs.python.org/issue1044
__Index: Doc/library/tarfile.rst
===
--- Doc/library/tarfile.rst (revision 57571)
+++ Doc/library/tarfile.rst (working copy)
@@ -168,6 +168,12 @@
:attr:`TarFile.errorlevel`\ ``== 2``.
 
 
+.. exception:: SecurityError
+
+   Is a subclass of :exc:`ExtractError` and is raised when insecure pathnames
+   are found on extraction.
+
+
 .. exception:: HeaderError
 
Is raised by :meth:`frombuf` if the buffer it gets is invalid.
@@ -327,16 +333,22 @@
available.
 
 
-.. method:: TarFile.extractall([path[, members]])
+.. method:: TarFile.extractall([path[, members[, check_paths]]])
 
Extract all members from the archive to the current working directory or
-   directory *path*. If optional *members* is given, it must be a subset of the
-   list returned by :meth:`getmembers`. Directory information like owner,
-   modification time and permissions are set after all members have been 
extracted.
-   This is done to work around two problems: A directory's modification time is
-   reset each time a file is created in it. And, if a directory's permissions 
do
-   not allow writing, extracting files to it will fail.
+   directory *path*. If optional *members* is given, it must be an iterator of
+   :class:`TarInfo` objects (e.g. a subset of the list returned by
+   :meth:`getmembers`). If *check_paths* is :const:`True` (default), insecure
+   pathnames that are absolute or point to a destination outside of the
+   archive's scope are rejected. Depending on :attr:`TarFile.errorlevel` a
+   :exc:`SecurityError` is raised.
 
+   Directory information like owner, modification time and permissions are set
+   after all members have been extracted. This is done to work around two
+   problems: A directory's modification time is reset each time a file is
+   created in it. And, if a directory's permissions do not allow writing,
+   extracting files to it will fail.
+
.. versionadded:: 2.5
 
 
@@ -349,9 +361,8 @@
 
.. note::
 
-  Because the :meth:`extract` method allows random access to a tar archive 
there
-  are some issues you must take care of yourself. See the description for
-  :meth:`extractall` above.
+  The :meth:`extract` method does not take care of several extraction 
issues.
+  In most cases you should consider using the :meth:`extractall` method.
 
 
 .. method:: TarFile.extractfile(member)
Index: Lib/tarfile.py
===
--- Lib/tarfile.py  (revision 57571)
+++ Lib/tarfile.py  (working copy)
@@ -340,6 +340,9 @@
 class ExtractError(TarError):
 General exception for extract errors.
 pass
+class SecurityError(ExtractError):
+Exception for insecure pathnames.
+pass
 class ReadError(TarError):
 Exception for unreadble tar archives.
 pass
@@ -2006,12 +2009,13 @@
 
 self.members.append(tarinfo)
 
-def extractall(self, path=., members=None):
+def extractall(self, path=., members=None, check_paths=True):
 Extract all members from the archive to the current working
directory and set owner, modification time and permissions on
directories afterwards. `path' specifies a different directory
to extract to. `members' is optional and must be a subset of the
-   list returned by getmembers().
+   list returned by getmembers(). If `check_paths' is True insecure
+   pathnames are not extracted.
 
 directories = []
 
@@ -2019,6 +2023,20 @@
 members = self
 
 for tarinfo in members:
+if check_paths:
+try:
+self._check_path(tarinfo.name)
+if tarinfo.islnk():
+self._check_path(tarinfo.linkname)
+if tarinfo.issym():
+self._check_path(os.path.join(tarinfo.name, 
tarinfo.linkname))
+except SecurityError, e:
+if self.errorlevel  1:
+raise
+else:
+self._dbg(1, tarfile: %s % e)
+continue
+
 if 

[issue1044] tarfile insecure pathname extraction

[jan matejek]

jan matejek added the comment:

no change to extract() ?

otherwise looks good to me. if you don't object, i am applying this to
SUSE's python 2.5

__
Tracker [EMAIL PROTECTED]
http://bugs.python.org/issue1044
__
___
Python-bugs-list mailing list 
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1044] tarfile insecure pathname extraction

[Lars Gustäbel]

Lars Gustäbel added the comment:

In principle I do not object, but this is a preliminary patch. I am
still not happy with the naming of the check_paths argument. Also, the
patch was made against the trunk which means that it contains hunks with
the new reStructuredText documentation. Please be patient.

I do not change extract() because it has become more and more a
low-level method over the years, that makes promises it cannot keep and
should not be used at all. I try to discourage its use in the documentation.

__
Tracker [EMAIL PROTECTED]
http://bugs.python.org/issue1044
__
___
Python-bugs-list mailing list 
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com