[prev in list] [next in list] [prev in thread] [next in thread] 

List:       spamassassin-users
Subject:    Re: CVE-2018-11805 fix and sa-exim
From:       Henrik K <hege () hege ! li>
Date:       2019-12-18 15:30:38
Message-ID: 20191218153038.GA1870 () hege ! li
[Download RAW message or body]

On Wed, Dec 18, 2019 at 03:57:44PM +0100, Marco Gaiarin wrote:
> 
> Looking at the plugin code, the culprit come from:
> 
>     $optionhash  =~ s/;/,/g;
>     # This is safe, right? (users shouldn't be able to set it in their config)
>     %option=eval $optionhash;
> 
> So seems to me that the CVE fix 'broke' the options handling of the
> plugin ('eval' is not permitted anymore?!).

No, SA actually fixed what was broken before.  Evaling an unchecked string
is extremely dangerous.  If someone can change that string or is able to add
a new eval:greylisting rule somewhere, they are free to run any perl code or
system commands they like.

"This is safe, right?" - one should never even write something like this. 
Atleast don't publish stuff like that for others to use.  Yes it's from
2006, but still..

> If i substitute the former with:
>     #$optionhash  =~ s/;/,/g;
>     # This is safe, right? (users shouldn't be able to set it in their config)
>     #%option=eval $optionhash;
>     %option = ('dir' => '/var/spool/sa-exim/tuplets',
>         'method' => 'dir',
>         'greylistsecs' => '1800',
>         'dontgreylistthreshold' => 10,
>         'connectiphdr' => 'X-SA-Exim-Connect-IP',
>         'envfromhdr' => 'X-SA-Exim-Mail-From',
>         'rcpttohdr' => 'X-SA-Exim-Rcpt-To',
>         'greylistnullfrom' => 1,
>         'greylistfourthbyte' => 0 );
>     $self->{'rangreylisting'}=1;
> 
> the plugin works as expected.

It has always been required to use untaint_var to untaint things.

$optionhash = Mail::SpamAssassin::Util::untaint_var($optionhash);

This should be used _after_ the string is sanitized and verified as safe to
use.  But in this case, using eval is most unneeded and horrid way of
reading configuration, so it should not be used.  I suggest you leave your
changes as is and maybe look for modern plugins or ways to do your
greylisting..

[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic