[ archivemail-Patches-1874868 ] Allow writing to world-writable directory with sticky bit

[SourceForge.net <no...@so...>]

Patches item #1874868, was opened at 2008-01-18 17:58
Message generated for change (Comment added) made by nikosch
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=456912&aid=1874868&group_id=49630

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: None
Group: None
>Status: Closed
>Resolution: Fixed
Priority: 5
Private: No
Submitted By: Tim O'Donnell (tim_odonnell)
Assigned to: Nikolaus Schulz (nikosch)
Summary: Allow writing to world-writable directory with sticky bit

Initial Comment:
Current behavior:
If the destination directory is world-writable, halt.

New behavior:
If the destination directory is world-writable, then proceed only if the directory's sticky bit is set.

Rationale:
If the destination directory is world-writable with the sticky bit set, then only the user who owns the archived file (or the owner of the directory) can delete it. From the user's perspective, it's equivalent to the directory not being world-writable, so there's no need for concern.

I'm including an svn diff of the quick change. If you'd like this another way, just let me know.

Thanks!

Tim



----------------------------------------------------------------------

>Comment By: Nikolaus Schulz (nikosch)
Date: 2010-08-09 12:53

Message:
archivemail 0.8.0 does no longer bail out when the destination directory is
world-writable, and /tmp works just fine. Not that in order to do this
safely, there are some new security checks, which disallow symlinked target
files.

----------------------------------------------------------------------

Comment By: Nikolaus Schulz (nikosch)
Date: 2008-01-19 01:57

Message:
Logged In: YES 
user_id=1594781
Originator: NO

I do not feel comfortable applying this patch. 

archivemail's file operations are quite sloppy, it uses high-level
utilities (the shutil module) and thereby pretty much trusts its
environment. Actually, I think archivemail needs be fixed here, but it is
not trivial. As matters stand, there are at least two potentially raceable
spots: when archivemail reads an already existing archive, and when it
writes the new archive. 

Consider this example: the destination directory is a sticky /tmp, no
archive exists yet.  archivemail starts, archiving to a safe temporary
file. Note that the environment variables $TMPDIR, $TEMP, and $TMPDIR all
override /tmp as a place for temporary files. Now when archivemail's done,
it first tries to rename the tempfile to the final destination, which is
safe, albeit not entirely without problems (it silently overwrites an
existing archive, which you might just be reading, if it already existed
before). But this e.g. fails if the temporary file and /tmp are not on the
same filesystem; in this case, archivemail essentially does a 'cp -p
<tempfile> <dest>', which follows symlinks! So an attacker could easily
drop a symlink in /tmp and cause archivemail to overwrite the link target.


Similarly, an attacker can inject any file's content into the archive when
archivemail tries to read an existing archive in /tmp. So, until this
sloppiness is fixed, I prefer to leave the current check alone. 

One might argue, of course, that it's not smart to try to stop the user
from doing dangerous things like that. But then, I think it makes more
sense to remove the entire check than to special-case the /tmp scenario.

Nikolaus


----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=456912&aid=1874868&group_id=49630