Bug 2536 - vpopmail/qmail code neither warning- nor 100% taint-safe
Summary: vpopmail/qmail code neither warning- nor 100% taint-safe
Status: RESOLVED FIXED
Alias: None
Product: Spamassassin
Classification: Unclassified
Component: spamc/spamd (show other bugs)
Version: 2.60
Hardware: PC Linux
: P3 normal
Target Milestone: 3.3.0
Assignee: Malte S. Stretz
URL:
Whiteboard:
Keywords:
: 2507 (view as bug list)
Depends on: 3517
Blocks: 2555
  Show dependency tree
 
Reported: 2003-10-01 08:03 UTC by cyrille
Modified: 2009-09-27 17:44 UTC (History)
3 users (show)



Attachment Type Modified Status Actions Submitter/CLA Status
the promised patch patch None Malte S. Stretz [HasCLA]
maillog before patch text/plain None cyrille [NoCLA]
maillog after patch text/plain None cyrille [NoCLA]
Proposed Patch patch None Sossi Andrej [NoCLA]
Corrected patch patch None Sossi Andrej [NoCLA]
untaint $dir patch None Manuel Mausz [NoCLA]

Note You need to log in before you can comment on or make changes to this bug.
Description cyrille 2003-10-01 08:03:02 UTC
in file /var/log/messages 
sorry, I've no more information.

Oct  1 16:54:38 k2 spamd[6586]: Use of uninitialized value in numeric gt (>) at
/usr/bin/spamd line 1126, <GEN55> line 2. 
Oct  1 16:54:38 k2 spamd[6586]: Use of uninitialized value in hash element at
/usr/bin/spamd line 1133, <ASSIGN> line 33. 
Oct  1 16:54:38 k2 spamd[6586]: Use of uninitialized value in hash element at
/usr/bin/spamd line 1133, <ASSIGN> line 33. 
Oct  1 16:54:38 k2 spamd[6586]: Use of uninitialized value in string ne at
/usr/bin/spamd line 1111. 
Oct  1 16:54:38 k2 spamd[6586]: Use of uninitialized value in pattern match
(m//) at /usr/bin/spamd line 660. 
O
Comment 1 cyrille 2003-10-01 09:55:55 UTC
so, I make more investigation and I'd a look at line 1104 of spamd 2.60.
it is :
 if ( $qmailu{$dom} ne "" ) {

after tracing the call I can see that :
 $dom is undef
and
 %qmailu has no keys

I'm running spamd on a RedHat 7.2 with Qmail 1.03 and qmail-scanner 1.20rc3
and I'm using virtual domains with vpopmail 5.2.1
Perhaps is why the hash %qmailu is empty ?
Comment 2 cyrille 2003-10-01 10:00:57 UTC
Sorry to be verbose, but I try to add more information...

Some of da spamassin or qmail-scanner is creating a folder is the filesystem root :
 /no such user qmailq@

when I'd installed qmail-scanner, I set option --qs-user to user qmailq
Comment 3 Malte S. Stretz 2003-10-01 18:17:37 UTC
the qmail code is crap. it's neither taint- nor warning-safe. working on this. 
Comment 4 cyrille 2003-10-06 06:26:42 UTC
hello,

you said "the qmail code is crap" :
perhaps qmail is bad, but it is a large used software. perhaps we have to find a
way to clean that warnings ?

you said "working on this"
do you mean "you're working on" or "I have to work on" ?

Please, can you be clearer, I may want to help.
what can we do ?

thanx to follow that storie ;o)
cyrille
Comment 5 Malte S. Stretz 2003-10-06 06:41:58 UTC
When I said "it is crap" I meant it is a dirty hack (don't want to insult the 
original author here but that's how it is :o). With "working on it" I meant I 
went and refactored it a great deal, getting rid of a bunch of uninitialized 
values and race conditions. That's not yet finished but I've planned to finish 
it this evening (yesterday evening even ;-). So the qmail/vpopmail code will 
definitely stay, just in better shape. 
 
Oh, and please don't change the milestone. 
Comment 6 cyrille 2003-10-06 06:48:34 UTC
Thanx Malte.

> Oh, and please don't change the milestone.

Sorry, I'm very sorry ;o{
Comment 7 Malte S. Stretz 2003-10-06 14:43:37 UTC
Created attachment 1463 [details]
the promised patch

Here it is. It's rather big because I had to refactor quite some stuff to get
rid of all those possible race conditions. Please try it out (I don't have
vpopmail and can test only till it tries to call vuserinfo).
Comment 8 cyrille 2003-10-07 00:55:26 UTC
thanx !!
I will try it this morning (2003-10-07)
Comment 9 cyrille 2003-10-07 13:13:52 UTC
Created attachment 1464 [details]
maillog before patch
Comment 10 cyrille 2003-10-07 13:14:16 UTC
Created attachment 1465 [details]
maillog after patch
Comment 11 cyrille 2003-10-07 13:14:37 UTC
I'm joining 2 files :
before_patch.txt => the maillog with spamd 2.60 NOT patched
after_patch.txt => the maillog with spamd 2.60 PATCHED with promised_patch

It still create a folder in filesystem root :
 /no\ such\ user\ qmailq\@

I repeat some config stuff :
- qmail-scanner is running as user qmailq
- spamd is running as root with options : "-D -d -a --vpopmail"
Comment 12 cyrille 2003-10-07 13:17:54 UTC
to get no more problem with folder creation
and config file loading
I've changed spamd start options to :
"-d -a --nouser-config --virtual-config=/etc/mail/spamassassin --vpopmail"

know spamd not create folder : /no\ such\ user\ qmailq\@
and it is finding its preference in file : 
 /etc/mail/spamassassin/default.prefs
where spamd has created a folder :
 /etc/mail/spamassassin/qmailq/.spamassassin

regards,
cyrille
Comment 13 Malte S. Stretz 2003-10-07 14:17:54 UTC
Hi cyrille, 
 
there seems to be something *very* wrong. Could you please add the following 
code in line 895 (just after the (getpwnam($userid))[2, 3, 7] line): 
if ($opt{'debug'}) { 
  warn "handle_user: name=$userid/$username id=$uid/$gid home=$userhome"; 
} 
and tell me the debug output this produces? it seems like there's some 
username/address ($username) fed to vuserinfo which does not exist and spamd 
can't handle this.  
Comment 14 Justin Mason 2003-11-01 16:42:52 UTC
I think we're going to have to punt on this change for 2.61.
Comment 15 Duncan Findlay 2003-11-11 20:05:45 UTC
3rd and 10... going for the punt.
Comment 16 Theo Van Dinter 2003-12-28 09:28:41 UTC
cyrille, any update on this ticket?  if not, I think we'll probably just apply 
the patch and close the ticket.
Comment 17 cyrille 2003-12-29 04:30:09 UTC
Sorry to be out.
I'm back now.
I will answer you tonigth after adding code after line 895.
cyrille
Comment 18 Theo Van Dinter 2004-01-13 22:00:03 UTC
I'm thinking that the patch is a bit much to digest into 2.6x.  how big an issue would it be to leave 
the changes for 2.70?
Comment 19 Malte S. Stretz 2004-01-13 22:38:05 UTC
Actually, not too much. If the qmail code in 2.60 worked, the users will still 
have the nasty message but it works. And if it didn't they had to wait for the 
next release which "supports" qmail anyway ;-) Sorry, cyrille. 
Comment 20 Malte S. Stretz 2004-01-24 18:29:09 UTC
Gentoo bug 37280 is probably related to this. 
  http://bugs.gentoo.org/show_bug.cgi?id=37280 
Will check this for 2.70. 
Comment 21 Daniel Quinlan 2005-03-30 01:08:53 UTC
move bug to Future milestone (previously set to Future -- I hope)
Comment 22 Justin Mason 2006-10-19 06:31:55 UTC
vpopmail note: if anyone who runs vpopmail would like to fix the assorted bugs
in spamd's support for it, and update the documentation to be correct, please
have at it.  We currently have a number of bugs: bug 2536, bug 4568, bug 4714,
bug 3120 and bug 2866, all related to vpopmail, and none of the dev team use
this code, so the various proposed fixes are untested. Basically, we need a
vpopmail maintainer.
Comment 23 Justin Mason 2006-10-19 06:57:06 UTC
didn't mean to close the bug
Comment 24 Sossi Andrej 2009-02-11 03:41:54 UTC
Created attachment 4432 [details]
Proposed Patch

I wrote a patch befor for Spamassassin 3.2.1 and now for Spamassassin 3.2.5. which should solve the problems with the aliases. The patch searches for the first real recipient it finds in the .qmail files. It looks through every
alias until it finds the first local recipient and sets the home
directory in this user in order to create the user_prefer.
There's a command in the patch to avoid indefinite iterations (eg.
alias1->alias2->alias1...). If it doesn't find real users (for example
if it only finds forwards to other programs or remote em-mails and not
local domains), from home directory it gets set to undefined.
We tested the patch for about 1 year on the Spamasassin 3.2.1 and now we
testing on the Spamassassin 3.2.5. On the production server seems work
fine.
Since I'm a new PERL user I apologize in advance if I haven't followed
the style rules elected from the Spamassassin dev team. So I beg you to
let me know eventual style errors in order to able to correct them.
I also kindly ask you to include the patch in the following versions of
Spamassassin.

Thanks
Comment 25 Sossi Andrej 2009-04-14 02:26:56 UTC
Is it possible to plan my patch's insert (and test) in the next version of Spamassassin? Is it possible to set Target Milestone at 3.3.0?
Thanks.
Comment 26 Justin Mason 2009-04-14 02:36:21 UTC
(In reply to comment #25)
> Is it possible to plan my patch's insert (and test) in the next version of
> Spamassassin? Is it possible to set Target Milestone at 3.3.0?
> Thanks.

I'll aim it at 3.3.0, but the vpopmail support is basically rotting due to the lack of a competent maintainer, so there's a high probability it won't be applied in time...
Comment 27 Justin Mason 2009-06-29 04:23:43 UTC
can anyone test this?  I've asked the users@ list to test, and if it's ok I'll apply it.
Comment 28 Justin Mason 2009-06-29 04:24:29 UTC
lowering pri without testers
Comment 29 Kevin Golding 2009-06-29 08:40:38 UTC
Jun 29 15:35:03 offa spamd[63128]: Use of uninitialized value in concatenation (.) or string at /usr/local/bin/spamd line 2113, <GEN164> line 2.
Jun 29 15:35:03 offa spamd[63128]: Use of uninitialized value in concatenation (.) or string at /usr/local/bin/spamd line 2125, <GEN164> line 2.
Jun 29 15:35:03 offa spamd[63128]: Use of uninitialized value in pattern match (m//) at /usr/local/bin/spamd line 2166, <GEN164> line 2.

Doesn't seem to fix it for me.
Comment 30 Sossi Andrej 2009-06-29 09:50:59 UTC
(In reply to comment #29)
> Jun 29 15:35:03 offa spamd[63128]: Use of uninitialized value in concatenation
> (.) or string at /usr/local/bin/spamd line 2113, <GEN164> line 2.
> Jun 29 15:35:03 offa spamd[63128]: Use of uninitialized value in concatenation
> (.) or string at /usr/local/bin/spamd line 2125, <GEN164> line 2.
> Jun 29 15:35:03 offa spamd[63128]: Use of uninitialized value in pattern match
> (m//) at /usr/local/bin/spamd line 2166, <GEN164> line 2.
> 
> Doesn't seem to fix it for me.

What patch did you tested? The line 2113 is a comment if applies patch to version 3.2.5 of Spamassassin.
Tomorrow I send the complete list of tests that I run with the most combinations of aliases that I used.
Comment 31 Kevin Golding 2009-06-29 10:01:02 UTC
Patch 4432 against SA 3.2.5 - admittedly one installed through FreeBSD Ports but the only patch that applies to spamd is http://www.freebsd.org/cgi/cvsweb.cgi/ports/mail/p5-Mail-SpamAssassin/files/patch-spamd_spamd.raw?rev=1.3

Line 2113 for me reads:

$dir = `$vpopdir/bin/vuserinfo -d \Q$username\E`;

2125:

$vpopalias = `$vpopdir/bin/valias \Q$username\E`;

And 2166:

if ($#todo == -1 && $work !~ /[a-z0-9_-]+(\.[a-z0-9_-]+)*@[a-z0-9_-]{2,}(\.[a-z0-9_-]+)*\.[a-z]{2,4}/) {
Comment 32 Sossi Andrej 2009-06-29 11:06:01 UTC
Weird. I also use SpamAssassin from ports of Freebsd. I did the installation on February 3 but I do not see major differences in the port so far.
My configuration was:
web # make showconfig
===> The following configuration options are available for p5-Mail-SpamAssassin-3.2.5_1:
      AS_ROOT = on "Run spamd as root (recommended)"
      Spamc = on "Build spamd / spamc (not for amavisd)
      SACOMPILE = on "sa-compile"
      DKIM = on "DKIM / DomainKeys Identified Mail"
      SSL = off "Build with SSL support for spamd / spamc"
      GNUPG = on "Install GnuPG (for sa-update)"
      MYSQL = off "Add MySQL support"
      Pgsql = off "Add PostreSQL support"
      RAZOR = on "Add Vipul's Razor support"
      SPF_QUERY = on "Add SPF query support"
      RELAY_COUNTRY = on "Relay country support"
===> Use 'make config' to modify these settings

The patch works fine by February without errors or warnings in the logs.

I try to investigate tomorrow.
Comment 33 Sossi Andrej 2009-07-06 02:28:29 UTC
I'm sorry for the delay, but I had lots of work to do and I didn't have the
chance to answer before.
While checking the source code, the pointed out error is generated when the
username isn't set.
It never happened to me that the username (e-mail address) hasn't been set
before entering in my patch. It's strange. I modified the patch to avoid the
error. If the username doesn't improve the function the user_prefers can't
be applied.
Thank you very much for the tests.

Best regards.
Comment 34 Sossi Andrej 2009-07-06 02:31:01 UTC
Created attachment 4471 [details]
Corrected patch

Avoid warning message if username is not set.
Comment 35 Kevin Golding 2009-07-06 05:55:40 UTC
I can't recreate the errors this time, looks good to me.

+1 for inclusion
Comment 36 Justin Mason 2009-07-06 06:30:09 UTC
great! I'll apply this later. thanks for testing it
Comment 37 Manuel Mausz 2009-09-07 11:33:46 UTC
Created attachment 4532 [details]
untaint $dir

Patch works fine but you need to untaint $dir so mkdir works.
Comment 38 Mark Martinec 2009-09-17 16:47:22 UTC
> Patch works fine but you need to untaint $dir so mkdir works.

Bug 6206, Bug 2536: spamd: untaint directory as obtained from
a password file or from vpopmail utilities, avoid implicit
untainting; report error if user preferences file exists
but cannot be accessed; some cosmetics (avoid deep nesting
where possible, and avoid faraway small code segments)
  Sending        lib/Mail/SpamAssassin.pm
  Sending        spamd/spamd.raw
Committed revision 816412.


Is this now fixed?
Comment 39 Mark Martinec 2009-09-18 16:23:57 UTC
*** Bug 2507 has been marked as a duplicate of this bug. ***
Comment 40 Mark Martinec 2009-09-21 03:35:21 UTC
fixed