On Wed, Dec 18, 2019 at 05:30:38PM +0200, Henrik K wrote:
> 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..
I've reported this to atleast Debian and Ubuntu along with a proper fix.
