Comment 40 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-28 07:38:42 -0000, Martin Pitt:
> Indeed that's the same case for the stable patches I proposed. All paths
> delivered by find start with /var/crash/, so the -- isn't strictly
> needed. OTOH it does not really hurt to have it, so if you upload the
> debdiffs, please feel free to add them.

Yes, that -- is not necessary, it makes the command line longer,
with no additional benefit but appart from that doesn't hurt.

> As for "use an existing program", /lib/init.d/bootclean.sh does
>
> find . -depth -xdev $TEXPR $EXCEPT ! -type d \
> -print0 | xargs -0r rm -f -- \
> || { report_err ; return 1 ; }

I hope that bootclean stuff is not called after a user may log
in the system as it has the same problem as that reported in
this bug.

It's also got a bug:

On intrepid at least, EXCEPT is defined as:

        EXCEPT='! -name .
                ! ( -path ./lost+found -uid 0 )
                ! ( -path ./quota.user -uid 0 )
                ! ( -path ./aquota.user -uid 0 )
                ! ( -path ./quota.group -uid 0 )
                ! ( -path ./aquota.group -uid 0 )
                ! ( -path ./.journal -uid 0 )
                ! ( -path ./.clean -uid 0 )
                ! ( -path './...security*' -uid 0 )'

Those inner "'" have no effect, but upon expansion of $EXCEPT,
filename generation will be performed on the "./...security*"
word resulting for word splitting. If there is more than one
file matching that pattern, that will cause find to fail.
The fix would be to do:

(set -f; find ... $EXCEPT ...)

to prevent filename generation upon expansion of $EXCEPT.

Also, it doesn't stop find from descending into lost+found
(which is a directory anyway so wouldn't be selected).

The fix would be:

        EXCEPT='! -name .
                ! ( -path ./lost+found -uid 0 -prune )
                ! ( -path ./quota.user -uid 0 )
                ! ( -path ./aquota.user -uid 0 )
                ! ( -path ./quota.group -uid 0 )
                ! ( -path ./aquota.group -uid 0 )
                ! ( -path ./.journal -uid 0 )
                ! ( -path ./.clean -uid 0 )
                ! ( -path ./...security* -uid 0 )'

And remove "-depth"

Note that -path is a very recent addition to the POSIX
specification (SUSv4). -print0/-0r are not standard.

[...]
> Thus my updated find expression
>
> find /var/crash -mindepth 1 -maxdepth 1 \( -type f -o -type l \)
> -mtime +7 -print0 | xargs -0 rm -f
>
> is much stricter than the bootclean.sh one, except for the --.

Agreed.

> I still think it is sufficient.

Agreed.

Cheers,
Stephane