DecodeShortURLs database breaks with setuid spamd

[Amir Caspi <ce...@3phase.com>]

Hi all,

	Just FYI, for those of you who use DecodeShortURLs.pm ... it appears that, if you are running in a per-user setup (i.e., running spamd as root such that it does a setuid <user> when invoked from spamc, and/or allowing individual users to run spamassassin), then the short-URL cache used by DSU must be world-writable (or, more correctly, writable by all users invoking spamd/SA), else the DSU rules fail.  This is because DSU is trying to write to the cache file; if it appears read-only to spamd/SA, the HAS_SHORT_URL rule fails, which halts DSU operation.

	Unfortunately the error is completely silent unless running in debug mode, and since very few people operate spamd that way (or even SA, in normal usage), the only way one would notice this is because the rule fails to fire when it should.  Sysadmins who normally use spamd as root, but check their installation via manually invoking SA as root, could potentially miss this.

I filed an issue at DSU's github:
https://github.com/smfreegard/DecodeShortURLs/issues/6

The workaround for now, if running per-user, is to make the DB world-writable.  The ultimate solution would be to fail gracefully (ignore failure to write; treat failure to read as "not found"; perform appropriate lookups even if not writable), and to provide an option for per-user databases, like with Bayes.

Unfortunately I'm not at a position where I can patch the code myself, but I wanted to let SA list users know, in case they might be in the same boat.

Cheers.

--- Amir

Re: DecodeShortURLs database breaks with setuid spamd

[Amir Caspi <ce...@3phase.com>]

On Mar 6, 2018, at 5:19 PM, RW <rw...@googlemail.com> wrote:
> 
> Or probably more commonly when running  the spamassassin perl script as
> an ordinary user for test purposes.

Right, if the DB is owned by that user, then they would see the rule fire with spamassassin and might assume it's all good for everyone.  If the DB is owned by root/nobody/some other user, but they test as a regular non-owner user, the rule should fail and they might notice it... and then give up on fixing it because it's an obscure problem to track down! =)

> Actually it does get cached results from the DB and does fall back to
> doing its own lookup. The debug is a little misleading because it tries
> either to update the timestamp or create a new entry before it even logs
> the result, and that's where it fails.

Gotcha. I didn't see the lookups happening but maybe I wasn't looking closely enough.  But, clearly, the failure to write appears to stop things in their tracks and the HAS_SHORT_URL rule never triggers, which causes everything downstream to also miss.

> I don't really understand the database interface, but if the two
> relevant  execute commands are placed in eval blocks it seems to work.
> cache_add() does use an eval block for setting-up the SQL, but the
> execute is outside the block.

That should be an easy patch, then.  Please submit on git!  I'm not sufficiently familiar with git yet...

But this only takes care of item #1: failing gracefully.  It no longer blocks but still fails to write to the DB.  So item #2 (per-user DBs) would still be needed for a fully robust fix, and I imagine that's more than a few lines of code, unfortunately.

Thanks for testing!

--- Amir

Re: DecodeShortURLs database breaks with setuid spamd

[RW <rw...@googlemail.com>]

On Mon, 5 Mar 2018 19:27:02 -0700
Amir Caspi wrote:

> Hi all,
> 
> 	Just FYI, for those of you who use DecodeShortURLs.pm ... it
> appears that, if you are running in a per-user setup (i.e., running
> spamd as root such that it does a setuid <user> when invoked from
> spamc, and/or allowing individual users to run spamassassin), then
> the short-URL cache used by DSU must be world-writable (or, more
> correctly, writable by all users invoking spamd/SA), else the DSU
> rules fail.  This is because DSU is trying to write to the cache
> file; if it appears read-only to spamd/SA, the HAS_SHORT_URL rule
> fails, which halts DSU operation.
> 
> 	Unfortunately the error is completely silent unless running
> in debug mode, and since very few people operate spamd that way (or
> even SA, in normal usage), the only way one would notice this is
> because the rule fails to fire when it should.  Sysadmins who
> normally use spamd as root, but check their installation via manually
> invoking SA as root, could potentially miss this.

Or probably more commonly when running  the spamassassin perl script as
an ordinary user for test purposes.  

> I filed an issue at DSU's github:
> https://github.com/smfreegard/DecodeShortURLs/issues/6
> 
> The workaround for now, if running per-user, is to make the DB
> world-writable.  The ultimate solution would be to fail gracefully
> (ignore failure to write; treat failure to read as "not found";
> perform appropriate lookups even if not writable), 

Actually it does get cached results from the DB and does fall back to
doing its own lookup. The debug is a little misleading because it tries
either to update the timestamp or create a new entry before it even logs
the result, and that's where it fails.

I don't really understand the database interface, but if the two
relevant  execute commands are placed in eval blocks it seems to work.
cache_add() does use an eval block for setting-up the SQL, but the
execute is outside the block.