Mailing List Archive: Python: Bugs

[issue4489] shutil.rmtree is vulnerable to a symlink attack

1 2

View All

Index | Next | Previous | View Threaded

report at bugs

Jun 9, 2012, 4:13 AM

Post #1 of 37 (270 views)
Permalink

[issue4489] shutil.rmtree is vulnerable to a symlink attack

Hynek Schlawack <hs [at] ox> added the comment:

Martin, are you still committed to this? I still think code duplication is bad (especially for security related code) but I’d be willing to write a fwalk-less version so it doesn’t look like I’m just defending my code here.

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue4489>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

Jun 9, 2012, 5:26 AM

Post #2 of 37 (266 views)
Permalink

[issue4489] shutil.rmtree is vulnerable to a symlink attack [In reply to]

Martin v. Löwis <martin [at] v> added the comment:

> Martin, are you still committed to this?

Yes, I'll provide a patch RSN. I doubt that there will be that much code
duplication.

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue4489>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

Jun 10, 2012, 1:40 PM

Post #3 of 37 (262 views)
Permalink

[issue4489] shutil.rmtree is vulnerable to a symlink attack [In reply to]

Martin v. Löwis <martin [at] v> added the comment:

Here is a patch with just the shutil changes. Compared to rmtree-with-fwalk-v3.diff, this changes 90 lines of rmtree, whereas the fwalk version changes only 70 lines. On the plus side, it's much more obvious in this version that the *at variant has the same algorithm as the non-*at version.

----------
Added file: http://bugs.python.org/file25935/direct_rmtree_safe.diff

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue4489>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

Jun 11, 2012, 4:47 AM

Post #4 of 37 (264 views)
Permalink

[issue4489] shutil.rmtree is vulnerable to a symlink attack [In reply to]

Hynek Schlawack <hs [at] ox> added the comment:

Fair enough, I'm not going to question your obviously superior judgement here. :)

However, your patch currently breaks the test suite on any platform that uses the fallback rmtree: You forgot the ignore_errors=False in the _rmtree_unsafe signature (and obviously also the argument when calling it as a fallback).

You also didn't seem to have touched the tests?

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue4489>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

Jun 18, 2012, 12:36 AM

Post #5 of 37 (260 views)
Permalink

[issue4489] shutil.rmtree is vulnerable to a symlink attack [In reply to]

Hynek Schlawack <hs [at] ox> added the comment:

Martin, what exactly is the intended proceeding now? Are you going to fix your patch and tests as soon as you have time or was that just a PoC and expect me/us to bring it into shape? (<- troll-free question, I have no idea what to do next here and would like to see a safe rmtree in 3.3)

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue4489>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

Jun 18, 2012, 12:53 AM

Post #6 of 37 (257 views)
Permalink

[issue4489] shutil.rmtree is vulnerable to a symlink attack [In reply to]

Martin v. Löwis <martin [at] v> added the comment:

> Martin, what exactly is the intended proceeding now? Are you going to
> fix your patch and tests as soon as you have time or was that just a
> PoC and expect me/us to bring it into shape? (<- troll-free question,
> I have no idea what to do next here and would like to see a safe
> rmtree in 3.3)

I still plan to work on this, but I'm also really really short on time.
I still favor my own approach (obviously), so if you want to bring it
into shape - that would be appreciated.

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue4489>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

Jun 21, 2012, 7:59 AM

Post #7 of 37 (253 views)
Permalink

[issue4489] shutil.rmtree is vulnerable to a symlink attack [In reply to]

Hynek Schlawack <hs [at] ox> added the comment:

I'll try to get it in before beta1 then.

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue4489>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

Jun 22, 2012, 10:28 AM

Post #8 of 37 (261 views)
Permalink

[issue4489] shutil.rmtree is vulnerable to a symlink attack [In reply to]

Hynek Schlawack <hs [at] ox> added the comment:

Here is my revised patch with Martin's code integrated.

Differences:

- fixed docs
- added some tests, test_on_errors could still use some refactorings though
- renamed _rmtree_safe to _rmtree_safe_fd so we can implement more of them
- _rmtree_safe_fd & _rmtree_unsafe don't need default arguments and the unsafe version also doesn't need argument normalization as both get onerror filled out by rmtree.

If nobody objects, I'd commit it soonish (definitely before Sunday).

----------
Added file: http://bugs.python.org/file26089/mvl-revisited-plus-docs.diff

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue4489>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

Jun 23, 2012, 9:18 AM

Post #9 of 37 (247 views)
Permalink

[issue4489] shutil.rmtree is vulnerable to a symlink attack [In reply to]

Roundup Robot <devnull [at] psf> added the comment:

New changeset c910af2e3c98 by Hynek Schlawack in branch 'default':
#4489: Add a shutil.rmtree that isn't suspectible to symlink attacks
http://hg.python.org/cpython/rev/c910af2e3c98

----------
nosy: +python-dev

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue4489>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

Jun 23, 2012, 11:30 AM

Post #10 of 37 (249 views)
Permalink

[issue4489] shutil.rmtree is vulnerable to a symlink attack [In reply to]

Roundup Robot <devnull [at] psf> added the comment:

New changeset 53fc7f59c7bb by Hynek Schlawack in branch 'default':
#4489: Fix usage of fd-based functions to new api introduced earlier today
http://hg.python.org/cpython/rev/53fc7f59c7bb

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue4489>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

Jun 23, 2012, 11:20 PM

Post #11 of 37 (248 views)
Permalink

[issue4489] shutil.rmtree is vulnerable to a symlink attack [In reply to]

Éric Araujo <merwok [at] netwok> added the comment:

Can I suggest setting a “safe” attribute on the rmtree function object instead of adding another name to the module?

----------
nosy: +larry

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue4489>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

Jun 23, 2012, 11:26 PM

Post #12 of 37 (248 views)
Permalink

[issue4489] shutil.rmtree is vulnerable to a symlink attack [In reply to]

Hynek Schlawack <hs [at] ox> added the comment:

I thought about that too but couldn't find a precedent (I didn't look very long though :)) where we've done that before so I went for an attribute. I'd change it immediately if others agree.

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue4489>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

Jun 23, 2012, 11:28 PM

Post #13 of 37 (247 views)
Permalink

[issue4489] shutil.rmtree is vulnerable to a symlink attack [In reply to]

Nick Coghlan <ncoghlan [at] gmail> added the comment:

I'm in the process of updating the LBYL support to use a "rmtree.avoids_symlink_attacks" function attribute rather than the "rmtree_is_safe" module level attribute.

As I said in the hmac.secure_compare function discussion, the words "safe" and "secure" are too vague to ever make for good API design. Much better to tell people exactly what they're safe against (rmtree_is_safe -> rmtree.avoids_symlink_attacks), or designed to be appropriate for (hmac.secure_compare -> hmac.compare_digest).

----------
nosy: +ncoghlan

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue4489>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

Jun 23, 2012, 11:31 PM

Post #14 of 37 (248 views)
Permalink

[issue4489] shutil.rmtree is vulnerable to a symlink attack [In reply to]

Hynek Schlawack <hs [at] ox> added the comment:

Excellent Nick! I discussed the very same thing with David yesterday but somehow we didn't come up with a good name. So we kept it on "safe" (which predates the secure discussion).

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue4489>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

Jun 23, 2012, 11:36 PM

Post #15 of 37 (248 views)
Permalink

[issue4489] shutil.rmtree is vulnerable to a symlink attack [In reply to]

Larry Hastings <larry [at] hastings> added the comment:

Bikeshedding:

(os.unlink in os.supports_dir_fd and os.open in os.supports_dir_fd)

could be rewritten as

{ os.open, os.unlink } <= os.supports_dir_fd

As you were!

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue4489>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

Jun 23, 2012, 11:42 PM

Post #16 of 37 (248 views)
Permalink

[issue4489] shutil.rmtree is vulnerable to a symlink attack [In reply to]

Éric Araujo <merwok [at] netwok> added the comment:

The code using set operations seems slightly too cryptic for me, even though I’m comfortable with sets and functions as first-class objects. A matter of taste I guess.

BTW it sounds a bit strange to me to have the verb in the singular in supports_dir_fd when its meaning is “the functions in this set support dir_fd”. I guess it’s too late to propose “os.open.supports_dir_fd and os.unlink.supports_dir_fd” (and I don’t know if that is feasible with C functions) :)

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue4489>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

Jun 23, 2012, 11:43 PM

Post #17 of 37 (249 views)
Permalink

[issue4489] shutil.rmtree is vulnerable to a symlink attack [In reply to]

Roundup Robot <devnull [at] psf> added the comment:

New changeset c2be81151994 by Nick Coghlan in branch 'default':
Issue #4489: Rename the feature marker for the symlink resistant rmtree and store it as a function attribute
http://hg.python.org/cpython/rev/c2be81151994

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue4489>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

Jun 23, 2012, 11:44 PM

Post #18 of 37 (248 views)
Permalink

[issue4489] shutil.rmtree is vulnerable to a symlink attack [In reply to]

Larry Hastings <larry [at] hastings> added the comment:

> I guess it’s too late to propose “os.open.supports_dir_fd and
> os.unlink.supports_dir_fd” (and I don’t know if that is feasible
> with C functions) :)

Where were you when "is_implemented" was being savagely torn apart last week? ;-)

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue4489>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

Jun 23, 2012, 11:46 PM

Post #19 of 37 (251 views)
Permalink

[issue4489] shutil.rmtree is vulnerable to a symlink attack [In reply to]

Éric Araujo <merwok [at] netwok> added the comment:

I was at work, and moving out of my apartment, and desperating at the subthreads spawned by my mail about packaging, and agreeing with the people being -1 on is_implemented on Signature objects :-)

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue4489>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

Jun 23, 2012, 11:46 PM

Post #20 of 37 (248 views)
Permalink

[issue4489] shutil.rmtree is vulnerable to a symlink attack [In reply to]

Nick Coghlan <ncoghlan [at] gmail> added the comment:

Éric - there's almost certainly going to be a PEP for 3.4 about doing this kind of feature advertisement in a cleaner and more consistent way.

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue4489>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

Jun 23, 2012, 11:48 PM

Post #21 of 37 (250 views)
Permalink

[issue4489] shutil.rmtree is vulnerable to a symlink attack [In reply to]

Éric Araujo <merwok [at] netwok> added the comment:

Yep, that is promising. At worst we’ll have a new cool API and three obsolete sets in the os module, not a big deal.

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue4489>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

Jun 24, 2012, 12:08 AM

Post #22 of 37 (249 views)
Permalink

[issue4489] shutil.rmtree is vulnerable to a symlink attack [In reply to]

Hynek Schlawack <hs [at] ox> added the comment:

Okay everyone, let's call it day – after 3,5 years. :) Further enhancement requests please in separate tickets. Thanks to everybody who took part!

----------
resolution: -> fixed
stage: patch review -> committed/rejected
status: open -> closed

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue4489>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

Jun 24, 2012, 4:27 PM

Post #23 of 37 (249 views)
Permalink

[issue4489] shutil.rmtree is vulnerable to a symlink attack [In reply to]

Arfrever Frehtes Taifersar Arahesis <Arfrever.FTA [at] GMail> added the comment:

The fix for this issue broke support for bytes in shutil.rmtree:

$ mkdir -p /tmp/a/b
$ python3.2 -c 'import shutil; shutil.rmtree(b"/tmp/a")'
$ mkdir -p /tmp/a/b
$ python3.3 -c 'import shutil; shutil.rmtree(b"/tmp/a")'
Traceback (most recent call last):
File "<string>", line 1, in <module>
File "/usr/lib64/python3.3/shutil.py", line 444, in rmtree
_rmtree_safe_fd(fd, path, onerror)
File "/usr/lib64/python3.3/shutil.py", line 381, in _rmtree_safe_fd
fullname = os.path.join(path, name)
File "/usr/lib64/python3.3/posixpath.py", line 78, in join
if b.startswith(sep):
TypeError: startswith first arg must be str or a tuple of str, not bytes
$

----------
priority: normal -> release blocker
resolution: fixed ->
stage: committed/rejected ->
status: closed -> open

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue4489>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

Jun 24, 2012, 4:52 PM

Post #24 of 37 (247 views)
Permalink

[issue4489] shutil.rmtree is vulnerable to a symlink attack [In reply to]

Larry Hastings <larry [at] hastings> added the comment:

> The fix for this issue broke support for bytes in shutil.rmtree:

What platform? Windows, or non-Windows? It'll probably be obvious regardless, but that might help.

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue4489>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

Jun 24, 2012, 8:18 PM

Post #25 of 37 (248 views)
Permalink

[issue4489] shutil.rmtree is vulnerable to a symlink attack [In reply to]

Nick Coghlan <ncoghlan [at] gmail> added the comment:

Tinkering with os.path.join, that traceback means that "name" is a str instance, while "path" is a bytes instance.

The culprit actually appears to be the fact that the type returned by os.listdir (et al) when handed a file descriptor is always a string (this is not explicitly documented, a problem in itself, but that's the behaviour I see here on linux).

One way to handle this would be to use the filesystem encoding with surrogateescape to decode any bytes path passed in to shutil.rmtree (at least in the _rmtree_safe_fd case) and handle the actual removal with Unicode throughout.

So long as the original encoding of the supplied bytes path is compatible with that of the underlying filesystem, surrogateescape should ensure that everything round trips correctly.

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue4489>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

1 2

View All

Index | Next | Previous | View Threaded

Interested in having your list archived? Contact Gossamer Threads