Comment 7 for bug 357024

Revision history for this message
Stephane Chazelas (stephane-chazelas) wrote : Re: [Bug 357024] Re: security hole in /etc/cron.daily/apport

2009-04-08 23:11:34 -0000, Martin Pitt:
> Stephane, I originally added -mindepth 1 to avoid deleting /var/crash/

Hi Martin, thanks for looking into that

note that rm -f wont remove directories, so it's only useful in
case /var/crash has been changed to a file or to avoid an error
message.

> itself in the cronjob. However, I think -type f -mindepth 1 -maxdepth 1
> is a safer way.

Yes, that's the non-portable equivalent of

find /var/crash/. ! -name . -prune -type f

> So for the security update I propose to change
>
> find /var/crash -mindepth 1 -mtime +7 -print0 | xargs -0 rm -f
> find /var/crash -mindepth 1 -empty -print0 | xargs -0 rm -f
>
> to
>
> find /var/crash -mindepth 1 -maxdepth 1 -type f -mtime +7 -print0 | xargs -0 rm -f
> find /var/crash -mindepth 1 -maxdepth 1 -type f -empty -print0 | xargs -0 rm -f
>
> for the sake of minimal and well-understood changes. I'll do the more
> standards friendly version you proposed upstream.

I forgot to mention that -print0 and xargs -0 were not standard
as well. To have the equivalent of -exec ... {} +, you also need
the (non standard) -r option to xargs to avoid running rm if no
file is found (though it's harmless)

Another advantage of

cd /var/crash &&
  find . ! -name . -prune -type f \( -size 0 -o -mtime +7 \) \
    -exec rm -f {} +

is that it makes smaller rm command lines (so potentially calls
fewer rm commands), also it avoid problems if /var/crash is
renamed or removed in the mean time (though, if that happens,
you're in serious trouble already I suppose).

> (Disclaimer: completely untested yet, I just confirmed that -type f does
> not catch symlinks)
[...]

Even if it did, rm -f would not remove the target of the link
but the link itself and -empty doesn't select symlinks and
-mtime checks the modification time of the link, not its target
(unless the -L option (former -follow predicate) is provided).

Another concern one may have (but that may just be nitpicking):
that script may remove files that are not /owned/ by apport,
which may become a problem if another package is using that
directory. So you may want to add a -name '*.crash' or maybe use
a directory named after the package name.

Best regards,
Stephane