[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

Roundup Robot 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

Hynek Schlawack 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 original

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

Larry Hastings 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 original call in shutil.

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

Hynek Schlawack 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 the link

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

Larry Hastings 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 shutil._use_fd_functions.

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

Changes by Hynek Schlawack : -- resolution: -> fixed stage: -> committed/rejected status: open -> closed ___ Python tracker ___ ___ P

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

Roundup Robot 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

Hynek Schlawack 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

Arfrever Frehtes Taifersar Arahesis 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.) $ mkdir -p /tmp/a/b $

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

Changes by Arfrever Frehtes Taifersar Arahesis : -- resolution: -> fixed stage: -> committed/rejected status: open -> closed ___ Python tracker ___ _

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

Roundup Robot 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

Larry Hastings 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 clear as mud, but t

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

Nick Coghlan 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

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

Larry 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

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

Arfrever Frehtes Taifersar Arahesis 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 recen

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

Hynek Schlawack 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

Éric Araujo 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 ___ __

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

Nick Coghlan 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

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

Éric Araujo 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 track

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

Larry 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? ;-) -- __

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

Roundup Robot 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

Éric Araujo 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 i

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

Larry 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

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

Hynek Schlawack 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

Nick Coghlan 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

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

Hynek Schlawack 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

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

Éric Araujo 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 __

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

Roundup Robot 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

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

Roundup Robot 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 tra

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

Hynek Schlawack 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

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

Hynek Schlawack added the comment: I'll try to get it in before beta1 then. -- ___ Python tracker ___ ___ Python-bugs-list mailing lis

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

Martin v. Löwis 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 woul

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

Hynek Schlawack 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 lik

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

Hynek Schlawack 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 obvi

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

Martin v. Löwis 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

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

Martin v. Löwis 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 __

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

Hynek Schlawack 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 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

Antoine Pitrou added the comment: > Martin proposed > to > re-implement os.fwalk inside of rmdir which I’m -1 > on. I agree that it isn't worth the hassle.

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

Hynek Schlawack 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

Hynek Schlawack 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 _safe_fwalk_rmtree a

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

Hynek Schlawack 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 docs? The

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

Hynek Schlawack 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: http://bugs.python.org/file25631/rmtree-wi

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

Petri Lehtinen 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 documentation currently stat

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

Hynek Schlawack 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 keywords: +needs

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

Changes by Hynek Schlawack : -- assignee: tarek -> dependencies: +fwalk breaks on dangling symlinks keywords: -patch stage: patch review -> needs patch ___ Python tracker ___ __

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

Changes by Arfrever Frehtes Taifersar Arahesis : -- nosy: +Arfrever ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscrib

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

Charles-François Natali 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 added the comment: Anybody working on this one? I’d give it a shot otherwise. -- ___ Python tracker ___ ___ Python-bug

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

Éric Araujo 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 doesn’t have these

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

Changes by Jesús Cea Avión : -- nosy: +jcea ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.o

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

Changes by Ross Lagerwall : -- dependencies: +Add a generic directory walker method to avoid symlink attacks ___ Python tracker ___ ___

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

Hynek Schlawack 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

Antoine Pitrou 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 to do with it

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

Hynek Schlawack 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 dependency? Or

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

Ross Lagerwall added the comment: Thanks Charles, I'll take your comments into account and take a look at making a general walker method. -- ___ Python tracker ___ _

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

Antoine Pitrou 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 ___ __

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

Charles-François Natali 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. Concerning this issue

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

Antoine Pitrou 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 chmodtree(), chownt

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

Charles-François Natali 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, os.AT_SYMLINK_NOFOLLOW).st_mode

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

Ross Lagerwall 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') Right. http://bugs.p

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

Éric Araujo 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') http://bugs.python.o

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

Ross Lagerwall 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

Changes by Petri Lehtinen : -- nosy: +petri.lehtinen ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail

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

Changes by Hynek Schlawack : -- nosy: +hynek ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.

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

Éric Araujo added the comment: I made two comments on rietveld but the email was rejected. -- ___ Python tracker ___ ___ Python-bugs-l

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

Changes by Antoine Pitrou : -- dependencies: +Add posix.fdlistdir, create Python wrappers for openat() and others ___ Python tracker ___ _

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

Ross Lagerwall 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

Antoine Pitrou added the comment: Le mercredi 05 janvier 2011 à 16:58 +, Ross Lagerwall a écrit : > Ross Lagerwall 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 followin

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

Ross Lagerwall 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: https://www.securecoding.cert.org/confluence/display/seccode/POS35-C.+Avoid+race+conditions

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

Changes by Ralf Schmitt : -- nosy: +schmir ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.or

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

Antoine Pitrou 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 symbolic link") +

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

Ross Lagerwall 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 of maintaining state by

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

Changes by Éric Araujo : -- nosy: +eric.araujo stage: -> needs patch versions: +Python 2.5 ___ Python tracker ___ ___ Python-bugs-list

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

K Richard Pixley 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 __

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

Tarek Ziadé 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 apply patches if we get a

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

Antoine Pitrou 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

Antoine Pitrou 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 opinion on it,

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

Mart Sõmermaa 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

Mart Sõmermaa 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? ___ Python t

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

Mart Sõmermaa 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

Changes by Mart Sõmermaa : Removed file: http://bugs.python.org/file12483/test_issue4489.sh ___ Python tracker ___ ___ Python-bugs-list mailing

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

Antoine Pitrou 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 "check inode the

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

Mart Sõmermaa 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 emerges, perhaps th

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

Mart Sõmermaa 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 to a path that c

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

Mart Sõmermaa 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: http://bugs.python.org/file12484/i

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

Changes by Mart Sõmermaa : Added file: http://bugs.python.org/file12483/test_issue4489.sh ___ Python tracker ___ ___ Python-bugs-list mailing l

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

Mart Sõmermaa 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: http://bugs.python.org/file12482

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

Antoine Pitrou 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 this possibil

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

Mart Sõmermaa 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 behaviou

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

Antoine Pitrou 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 behaviour and s

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

Antoine Pitrou 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 dir 3) otherwis

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

Mart Sõmermaa 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 tracker

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

Antoine Pitrou 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

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

Antoine Pitrou added the comment: What course of action do you suggest? First chmod 0700 on the directory? -- nosy: +pitrou ___ Python tracker ___ ___

[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 /r