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

Change by daniel hahler : -- nosy: +blueyed nosy_count: 18.0 -> 19.0 pull_requests: +21884 pull_request: https://github.com/python/cpython/pull/22968 ___ Python tracker ___

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

Hynek Schlawack h...@ox.cx added the comment: Thanks you for catching that! It seems we did something really stupid: followed symlinks. I’ve fixed that and added regression tests. This one is a facepalm gentlepeople. -- ___ Python tracker

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

Roundup Robot devn...@psf.upfronthosting.co.za added the comment: New changeset f9f798f1421e by Hynek Schlawack in branch 'default': #4489: Don't follow ever symlinks in rmtree http://hg.python.org/cpython/rev/f9f798f1421e -- ___ Python tracker

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

Changes by Hynek Schlawack h...@ox.cx: -- resolution: - fixed stage: - committed/rejected status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4489 ___

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

Larry Hastings la...@hastings.org added the comment: I'm not a security guy, but: shouldn't the os.unlink call when it isn't a directory specify follow_symlinks=False? And wouldn't it be safer if the os.rmdir() call also used dir_fd=? Additionally, I think you missed some stuff for

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

Hynek Schlawack h...@ox.cx added the comment: I'm not a security guy, but: shouldn't the os.unlink call when it isn't a directory specify follow_symlinks=False? os.unlink has no follow_symlinks argument. Imagine what would happen if you‘d do a os.unlink() on a link and it would just remove

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

Larry Hastings la...@hastings.org added the comment: I'm pretty busy right now, please open a ticket for listdir. _rmtree_safe_fd could remove the directory just after the recursive step using the parent's dirfd. Of course you'd also have to add a rmdir for the very-tippy-top after the

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

Hynek Schlawack h...@ox.cx added the comment: I'm pretty busy right now, please open a ticket for listdir. done _rmtree_safe_fd could remove the directory just after the recursive step using the parent's dirfd. Of course you'd also have to add a rmdir for the very-tippy-top after the

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

Roundup Robot devn...@psf.upfronthosting.co.za added the comment: New changeset 9134bb4d0578 by Hynek Schlawack in branch 'default': #4489: Use dir_fd in rmdir in _rmtree_safe_fd() http://hg.python.org/cpython/rev/9134bb4d0578 -- ___ Python tracker

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

Arfrever Frehtes Taifersar Arahesis arfrever@gmail.com added the comment: The fix (c910af2e3c98 + 53fc7f59c7bb) for this issue broke deletion of directories, which contain symlinks to directories. (Directories with symlinks to regular files or symlinks to nonexistent files are unaffected.)

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

Changes by Arfrever Frehtes Taifersar Arahesis arfrever@gmail.com: -- resolution: - fixed stage: - committed/rejected status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4489

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

Roundup Robot devn...@psf.upfronthosting.co.za added the comment: New changeset 2e2329aeb5c1 by Hynek Schlawack in branch 'default': #4489 Make fd based rmtree work on bytes http://hg.python.org/cpython/rev/2e2329aeb5c1 -- ___ Python tracker

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

Éric Araujo mer...@netwok.org 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 rep...@bugs.python.org

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

Hynek Schlawack h...@ox.cx 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. -- ___

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

Nick Coghlan ncogh...@gmail.com 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

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

Hynek Schlawack h...@ox.cx 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

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

Larry Hastings la...@hastings.org 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

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

Éric Araujo mer...@netwok.org 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

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

Roundup Robot devn...@psf.upfronthosting.co.za 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

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

Larry Hastings la...@hastings.org 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? ;-)

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

Éric Araujo mer...@netwok.org 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 :-) --

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

Nick Coghlan ncogh...@gmail.com 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 rep...@bugs.python.org

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

Éric Araujo mer...@netwok.org 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 rep...@bugs.python.org http://bugs.python.org/issue4489

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

Hynek Schlawack h...@ox.cx 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

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

Arfrever Frehtes Taifersar Arahesis arfrever@gmail.com 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)'

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

Larry Hastings la...@hastings.org 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

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

Nick Coghlan ncogh...@gmail.com 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

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

Larry Hastings la...@hastings.org added the comment: Your deduction is correct. listdir can't tell what the original argument type was based on the output--path_converter abstracts away those details. So it separately tests the type of the first argument. Staring at it again it's about as

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

Roundup Robot devn...@psf.upfronthosting.co.za 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

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

Roundup Robot devn...@psf.upfronthosting.co.za 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 --

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

Hynek Schlawack h...@ox.cx 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 -

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

Hynek Schlawack h...@ox.cx added the comment: I'll try to get it in before beta1 then. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4489 ___

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

Hynek Schlawack h...@ox.cx 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

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

Martin v. Löwis mar...@v.loewis.de 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

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

Hynek Schlawack h...@ox.cx 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

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

Martin v. Löwis mar...@v.loewis.de 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

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

Hynek Schlawack h...@ox.cx 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. --

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

Martin v. Löwis mar...@v.loewis.de 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 rep...@bugs.python.org

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

Hynek Schlawack h...@ox.cx added the comment: A small docs update: I don't list the possible functions for onerror at all anymore, as that list is likely to grow and is a PITA to maintain. People shouldn’t rely on a certain set of functions here. Martin proposed

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

Antoine Pitrou pit...@free.fr added the comment: Martin proposed http://mail.python.org/pipermail/python-dev/2012-May/119555.html to re-implement os.fwalk inside of rmdir which I’m -1 http://mail.python.org/pipermail/python-dev/2012-May/119556.html on. I agree that it isn't worth the

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

Martin v. Löwis mar...@v.loewis.de added the comment: I don't think that doing direct flistdir/fstatat calls would be more complex. We already have directory walking implemented (in default_rmtree), and I think the fd-based calls could nicely integrate with that. I'll provide a patch.

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

Hynek Schlawack h...@ox.cx added the comment: I’ve incorporated all the feedback (I hope). I had to refactor the tests slightly, but we have 100% code coverage for both versions of rmtree. Speaking of: I’ve renamed the default version of rmtree to _default_rmtree and _safe_rmtree to

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

Hynek Schlawack h...@ox.cx added the comment: I'm taking Charles-François' review comments here. 1. since fwalk() uses O(depth directory tree) file descriptors, we might run out of FD on really deep directory hierarchies. It shouldn't be a problem in practise Should I mention it in the

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

Hynek Schlawack h...@ox.cx added the comment: I've implemented a _safe_rmtree which gets used if os.fwalk() and os.unlinkat() are available. Test suite still passes in regression mode both on Mac (= no effect) and Linux. Let me know if I missed something. -- assignee: - hynek

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

Petri Lehtinen pe...@digip.org added the comment: +Same a rmtree but uses safe functions to avoid race conditions ^ typo +onerror(os.unlinkat, os.path.join(path, name), sys.exc_info()) +onerror(os.fwalk, path, sys.exc_info()) The

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

Hynek Schlawack h...@ox.cx added the comment: Thanks Petri. I've added the missing s in my repo and attach a proposed separate doc patch, I'd like reviewed by someone whose English is better than mine. :) -- nosy: +ezio.melotti, georg.brandl Added file:

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

Changes by Hynek Schlawack h...@ox.cx: -- assignee: tarek - dependencies: +fwalk breaks on dangling symlinks keywords: -patch stage: patch review - needs patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4489

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

Changes by Arfrever Frehtes Taifersar Arahesis arfrever@gmail.com: -- nosy: +Arfrever ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4489 ___

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

Charles-François Natali neolo...@free.fr added the comment: Anybody working on this one? I’d give it a shot otherwise. Go ahead. You could - should? - probably use the new os.fwalk() to walk directories in a safe maner. -- ___ Python tracker

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

Hynek Schlawack h...@ox.cx added the comment: Anybody working on this one? I’d give it a shot otherwise. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4489 ___

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

Changes by Jesús Cea Avión j...@jcea.es: -- nosy: +jcea ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4489 ___ ___ Python-bugs-list mailing list

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

Éric Araujo mer...@netwok.org added the comment: In case you want opinions on pathlib: I, for one, disliked Jason Orendorff’s path module, because it did not distinguish between pure string operations and I/O-inducing operations, and also because it duplicated os/os.path functions. Your API

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

Hynek Schlawack h...@ox.cx added the comment: What's the current state here? Anyone working on a solution or are we waiting how http://hg.python.org/features/pathlib/ will work out? If the consensus is to add a generic walker method, wouldn't be appropriate to open a new bug and add it as

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

Antoine Pitrou pit...@free.fr added the comment: What's the current state here? Anyone working on a solution or are we waiting how http://hg.python.org/features/pathlib/ will work out? Well, I am not working on that one, so waiting for it to work out might be optimistic :) I don't know what

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

Hynek Schlawack h...@ox.cx added the comment: What's the current state here? Anyone working on a solution or are we waiting how http://hg.python.org/features/pathlib/ will work out? Well, I am not working on that one, so waiting for it to work out might be optimistic :) I don't know

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

Changes by Ross Lagerwall rosslagerw...@gmail.com: -- dependencies: +Add a generic directory walker method to avoid symlink attacks ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4489 ___

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

Antoine Pitrou pit...@free.fr added the comment: I think the best thing would be to let rmtree fail (provided it closes all the FDs it opened) Agreed. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4489

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

Ross Lagerwall rosslagerw...@gmail.com added the comment: Thanks Charles, I'll take your comments into account and take a look at making a general walker method. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4489

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

Charles-François Natali neolo...@free.fr added the comment: FYI, I have a pathlib experiment in http://hg.python.org/features/pathlib/, with an optional openat-based accessor. Interesting: I used to think that the current API for dealing with paths was a little too basic and terse.

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

Charles-François Natali neolo...@free.fr added the comment: There's a race: --- Lib/shutil.py 2011-11-05 00:11:05.745221315 +0100 +++ Lib/shutil.py.new 2011-11-05 00:11:01.445220324 +0100 @@ -307,6 +307,7 @@ try: mode = os.fstatat(dirfd, name,

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

Antoine Pitrou pit...@free.fr added the comment: Finally, since writting a such code is tricky, what do you - all - think of making this a generic walker method that would take as argument the methods to call on a directory and on a file (or link), so that we could reuse it to write

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

Éric Araujo mer...@netwok.org added the comment: I made another review but my mail was rejected. http://bugs.python.org/review/4489/diff/3383/10563#newcode319 Lib/test/test_shutil.py:319: @unittest.skipIf(threading == None, 'requires threading') You can just say skipUnless(threading, 'msg')

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

Ross Lagerwall rosslagerw...@gmail.com added the comment: http://bugs.python.org/review/4489/diff/3383/10563#newcode319 Lib/test/test_shutil.py:319: @unittest.skipIf(threading == None, 'requires threading') On 2011/10/07 19:29:47, eric.araujo wrote: You can just say skipUnless(threading, 'msg')

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

Ross Lagerwall rosslagerw...@gmail.com added the comment: Updated patch based on Eric's comments: Store _supports_safe_rmdir at the module level. Move imports up to module level Skip test on non-threading build -- Added file: http://bugs.python.org/file23261/i4489_v4.patch

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

Éric Araujo mer...@netwok.org added the comment: I made two comments on rietveld but the email was rejected. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4489 ___

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

Changes by Hynek Schlawack h...@ox.cx: -- nosy: +hynek ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4489 ___ ___ Python-bugs-list mailing list

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

Changes by Petri Lehtinen pe...@digip.org: -- nosy: +petri.lehtinen ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4489 ___ ___ Python-bugs-list

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

Ross Lagerwall rosslagerw...@gmail.com added the comment: Here is a draft patch. It uses the *at functions and fdlistdir consequently it only makes it safe if those functions are available. It works using a recursive implementation and an open file descriptor pointing to a directory, instead

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

Antoine Pitrou pit...@free.fr added the comment: Thanks for the patch. There seems to be a race remaining here: +try: +if os.path.islink(path): +# symlinks to directories are forbidden, see bug #1669 +raise OSError(Cannot call rmtree on a

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

Changes by Ralf Schmitt sch...@gmail.com: -- nosy: +schmir ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4489 ___ ___ Python-bugs-list mailing

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

Ross Lagerwall rosslagerw...@gmail.com added the comment: Updated patch removes the race condition. Since an open follows symlinks, you can't just fstat the fd to see if it is a link. I followed the following to overcome this:

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

Antoine Pitrou pit...@free.fr added the comment: Le mercredi 05 janvier 2011 à 16:58 +, Ross Lagerwall a écrit : Ross Lagerwall rosslagerw...@gmail.com added the comment: Updated patch removes the race condition. Since an open follows symlinks, you can't just fstat the fd to see if it

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

Ross Lagerwall rosslagerw...@gmail.com added the comment: I think I misread the original implementation. Here is an updated version with that code just taken out. -- Added file: http://bugs.python.org/file20279/i4489_v3.patch ___ Python tracker

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

Changes by Antoine Pitrou pit...@free.fr: -- dependencies: +Add posix.fdlistdir, create Python wrappers for openat() and others ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4489 ___

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

Changes by Éric Araujo mer...@netwok.org: -- nosy: +eric.araujo stage: - needs patch versions: +Python 2.5 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4489 ___

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

K Richard Pixley r...@noir.com added the comment: How does rm -rf address this issue? Or does it? shutils.rmtree should probably do the same thing. -- nosy: +teamnoir ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4489

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

Tarek Ziadé ziade.ta...@gmail.com added the comment: Mart, I took over the maintenance of shutil, if you are interested in contributing a bullet-proof version of rmtree, please drop a line at python-ideas, and let's see what we can do in a new function maybe. I'll be happy to review and

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

Mart Sõmermaa m...@mrts.pri.ee added the comment: Ah, right you are. Attaching an initial alpha-quality patched shutil.py and a script to test the attack. Run the script by sourcing it with . test_issue4489.sh, not by executing (job control won't work in this case). Added file:

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

Changes by Mart Sõmermaa m...@mrts.pri.ee: Added file: http://bugs.python.org/file12483/test_issue4489.sh ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4489 ___

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

Mart Sõmermaa m...@mrts.pri.ee added the comment: And here's the diff so you can review what I was up to. Note that this does not yet fix the problem (although the logic looks about right), I have to examine the problem more thoroughly. -- keywords: +patch Added file:

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

Mart Sõmermaa m...@mrts.pri.ee added the comment: Aha, got it -- while removing /a/b/c/d, there's no easy way to detect that b or c has become a symlink. I.e. given directory tree a `-- b |-- c `-- d 1. os.rmdir('/a/b/c') succeeds 2. execution is suspended 3. '/a/b' is made a symlink

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

Mart Sõmermaa m...@mrts.pri.ee added the comment: A blunt, ineffective solution would be to walk the tree before removing it and recording path : inode pairs in a dict on first pass and then checking that the inodes have not changed during removal on second pass. If no clever bulletproof fix

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

Antoine Pitrou pit...@free.fr added the comment: A blunt, ineffective solution would be to walk the tree before removing it and recording path : inode pairs in a dict on first pass and then checking that the inodes have not changed during removal on second pass. There's no way to do the

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

Changes by Mart Sõmermaa m...@mrts.pri.ee: Removed file: http://bugs.python.org/file12483/test_issue4489.sh ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4489 ___

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

Mart Sõmermaa m...@mrts.pri.ee added the comment: Fixed a minor bug in test script and added Perl test as well. Perl with File-Path-2.07 passes the test. Added file: http://bugs.python.org/file12485/test_issue4489.sh ___ Python tracker

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

Mart Sõmermaa m...@mrts.pri.ee added the comment: Antoine, what if we add another function, rmtree_safe() that uses chdir() and document that it is protected from the race condition but may have the side effect of changing the current dir in threaded environment?

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

Antoine Pitrou pit...@free.fr added the comment: Antoine, what if we add another function, rmtree_safe() that uses chdir() and document that it is protected from the race condition but may have the side effect of changing the current dir in threaded environment? I don't have any strong

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

Mart Sõmermaa m...@mrts.pri.ee added the comment: Replying to previous comment: There's no way to do the check inode then remove sequence atomically. Right, although the attack window would be tiny, this is not a real solution. ___ Python tracker

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

Antoine Pitrou pit...@free.fr added the comment: FWIW, I've opened a separate bug entry for the creation of the openat(), etc. wrappers: #4761. Those functions seem to exist on recent Linux distros (even Debian stable). ___ Python tracker

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

Mart Sõmermaa m...@mrts.pri.ee added the comment: A shameless copy of the Perl fix for the bug http://bugs.debian.org/286922 looks like the evident solution. Somebody has to examine the fix though, I'm afraid I'm not currently able to do it. ___ Python

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

Antoine Pitrou pit...@free.fr added the comment: The Perl patch is here: http://bugs.debian.org/cgi-bin/bugreport.cgi?msg=36;filename=etch_03_fix_file_path;att=1;bug=286922 It is a recursive implementation of rmtree. What it does is 1) get the inode of the path 2) unlink it altogether if not a

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

Antoine Pitrou pit...@free.fr added the comment: Mmmh, the problem with Perl's approach is that it changes the current working directory (calls to chdir()), which is process-specific and not thread-specific. Currently, no function in shutil changes the current working directory, which is a nice

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

Mart Sõmermaa m...@mrts.pri.ee added the comment: Mmmh, the problem with Perl's approach is that it changes the current working directory (calls to chdir()), which is process-specific and not thread-specific. Currently, no function in shutil changes the current working directory, which is a

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

Antoine Pitrou pit...@free.fr added the comment: Using chdir() makes sense and it doesn't look like a too big problem to me: It's a problem if another thread in the process is making file operations using relative paths at the same time. Since shutil functions have until now been safe against

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

Antoine Pitrou pit...@free.fr added the comment: What course of action do you suggest? First chmod 0700 on the directory? -- nosy: +pitrou ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4489

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

Antoine Pitrou pit...@free.fr added the comment: Mmmh, very recent Linux kernels (= 2.6.16) seem to have specific functions to deal with this (see man pages for openat, unlinkat, etc.). ___ Python tracker rep...@bugs.python.org

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

New submission from Mart Sõmermaa [EMAIL PROTECTED]: Race condition in the rmtree function in the shutils module allows local users to delete arbitrary files and directories via a symlink attack. See also http://bugs.debian.org/286922 Attack: --- # emulate removing /etc $ sudo cp -a /etc