Re: Nasty hole in postifx/procmail/cyrus

[guenther () GAC EDU (Philip Guenther)]

John Pettitt <jpp () CLOUDVIEW COM> writes:
> There are a number of hacks about that allow postfix to deliver to cyrus
> imap mailboxes via procmail.    It turns out that at least one of these has
> a hole in it that allows bad guy to run code as the cyrus user.
...
> # this enables automated procmail recipe creation for users;
> # roll your own tool to allow creation of procmail recipes on a per-user
> # basis and place them there, but don't let users edit their own recipes
> INCLUDERC=/etc/procmailrcs/$1
>
> # make sure EXITCODE is clear: then it will take the value of the TRAP return
> EXITCODE=""
>
> TRAP="/usr/cyrus/bin/deliver -m $2 -- $1"
...
> When procmail processes the INCLUDERC and TRAP lines backquote expansion
> happens.  Sending a message  `shell commands`@myhost.com will cause the
> commands to run under whatever ID procmail happens to be running as
> (typically cyrus).

The backquote expansion takes place when TRAP is re-expanded for
processing (the Bourne shell's "trap" command suffers the same defect).
The INCLUDERC assignment is safe from them.  The correct assignment to
TRAP would delay variable expansion till TRAP is expanded, and
double-quote the user supplied variable even then to prevent filename
globbing and word breaking on spaces:

TRAP = '/usr/cyrus/bin/deliver -m "$2" -- "$1"'

(At least the original author got the "--" correct...)

> Also if mail is sent to baduser procmail will disclose the path to it's
> include dir in the reply.    Mail can then be set to say ../passwd which
> will case procmail to read passwd as a recipie file and barf it's contests
> as an error response.
>
> All of the above can be plugged by adding:
>
> :0
> *$ ! $MAILTO ?? .*[A-ZA-z0-9\-_]?
> /tmp/bad
> #or /dev/null according to taste
>
> before the INCLUDERC line in /etc/procmailrc

Heh, you just opened a new hole: the syntax to match the regexp against
a variable's value doesn't put a '$' before the variable name.  The
above will expand the value of MAILTO and then parse it as a condition,
allowing evil things if MAILTO starts with "?".  Hmm, your regexp is
also broken: it will _always_ match, so the condition will _never_
match.

The correct recipe would run something like:

        :0
        * MAILTO ?? [^-a-z0-9_]
        {
                # MAILTO contains a character that is neither alphanumeric,
                # nor an underbar, nor a minus sign.  We have no such users.
                EXITCODE = 67           # EX_NOUSER
                HOST
        }

That should go somewhere above the INCLUDERC assignment.

Philip Guenther