develooper Front page | perl.perl5.porters | Postings from January 2005

Re: Bug#286905: perl-modules: File::Path::rmtree makes setuid

Thread Previous | Thread Next
Aaron Sherman
January 12, 2005 14:02
Re: Bug#286905: perl-modules: File::Path::rmtree makes setuid
Message ID:
On Wed, 2005-01-12 at 07:45, Brendan O'Dea wrote:

> >Example of attack: suppose we know that root uses rmtree to clean up
> >/tmp directories.
> >Root would have recorded the permissions of /tmp/psz/sh, but would
> >"restore" it to /bin/sh.

I'll discuss this one, below with my reply to Brendan.

As a general response: the solution to this and any number of other
vulnerabilities is a system that presents filesystem multi-stage
transactions. That's not something Perl can rely on, but it's certainly
something Perl should support when it does exist (perhaps Reiser has
already gone there?)

Ok, now back to reality:

> >Following on from the "File::Path::rmtree makes setuid" issue, I notice
> >that rmtree may be tricked into removing arbitrary files.
> >
> >Example of attack: suppose we know that root uses rmtree to clean up
> >/tmp directories. Attacker prepares things:

You cited a general solution, and it's a good one. You have to chdir
into the directory and then readdir, restoring state as you go back up.

You also need to sanity check using Cwd and File::Spec before and after
the chdir to make sure that you didn't get hijacked.

> [p5p:] If anyone had a cleaner (and cross-platform) fix, I'd love to
> hear of it.

Well, certainly relying on rm (and you assumed a "-v" option which,
AFAIK implies GNU rm specifically) is right out. I'm sure others will
say the same.

Also, I don't see the value in changing permissions.

I understand WHY you want to do this, but it's far better to tell the
programmer calling rmtree that they might have to fix permissions before
calling rmtree. Otherwise, rmtree is doing exactly what your permissions
told it to, and you might have done that for a reason. Is it any worse
to have a developer say, "I made that directory read-only to all, but
rmtree nuked it!" vs. "I made that directory read-only to all, and
rmtree didn't nuke it!"? I can make that argument either way with just
as much sincerity and "oh my gods, you can't do this to me!" pleading as
I'm sure you've heard on this in the past... solution: punt.

Quick fix? Reduce the race by making any changes just before and just
after an operation, not in preparation for a whole directory. Now you
still have a problem, but a smaller one.

Further reduce the problem by using device/inode information from stat
on platforms where it is available. You can't make this atomic, but when
you need to do:

	if (! do_something()) {

checking just before revert_change() will reduce the race substantially.
Also, consider writing sanity-checking wrappers that handle all of this,
so that future additions don't have to boiler-plate your security code.

☎ 781-324-3772

Thread Previous | Thread Next Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at | Group listing | About