Re: Making pglister work with exim 4.96+

On Wed, Jun 19, 2024 at 11:29 AM Célestin Matte <celestin(dot)matte(at)cmatte(dot)me>
wrote:

> > Yeah, and I don't see why they would? The reason they do the taint
> marking in variables used in commands and filenames is that it would be a
> potential venue for attackers to inject things. No such vulnerability
> exists with environment variables. Obviously the receiving code, whether a
> shellscript or a python program or a c program or whatever, can have
> injection vulnerabilities of it's own, but the passing values layer (which
> is what Exim is responsible for there) does not.
>
> Yet this is what we want to do here: bypass security protection by passing
> dangerous data through environment variables. It would make sense for them
> to prevent that usage
>

How is the data dangerous?

I mean, we pass the body of the email to inject.py as well, and that's also
under control of the sender. And if that's a problem, they'd have to remove
the entire pipe transport completely, which I really doubt wil lhappen...

The data is only dangerous when potentially badly escaped for e.g. a
commandline. Passing it in an environment variable does not have that
problem.

> Yeah, this seems extremely fragile. Concurrent delivery is a common
> thing, and not the only potential problem I bet. The proper fix surely is
> to make invoke.py work properly.
>
> What's invoke.py? Do you mean inject.py?
>

Yes.

I'm aware of the potential concurrency issues. One fix could be to only
> process emails in mailqueuehandler.py if their sender address is not empty
> (or we could add a boolean field for that purpose).
>

I mean, yeah, we could. But it still adds a complexity and fragitlity, for
from what I can tell zero actual gain.

> And the above doesn't actually solve the problem does it? It still
> requires passing the message-id which is a tainted variable?
>
> $message_id is not the header, it's exim's internal message ID and is
> untainted.
>

Oh. But we need the actual message-id. Having the exim internal one is of
no value.

> Here's my current version, handling the header as well:
> event_action = ${if eq {msg:delivery}{$event_name} {${lookup pgsql{update
> incoming_mail set sender='${quote_pgsql:$sender_address}',
> messageid='${quote_pgsql:$header_message-id:}' where
> messageid='${quote_pgsql:$message_id}'; notify incoming; update bounce_mail
> set sender='${quote_pgsql:$sender_address}',
> messageid='${quote_pgsql:$header_message-id:}' where
> messageid='${quote_pgsql:$message_id}'; notify bounce}} {}}}
>
>
> Another overall solution may be to fetch header_message-id and
> sender_address from exim in inject.py using a subprocess (provided it's
> still queued at that point?)
>

I still completely fail to see why this complexity is a good idea.

--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/>
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>