You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@spamassassin.apache.org by Ben Lentz <bl...@channing-bete.com> on 2006/10/10 20:39:04 UTC

Mail::DomainKeys::Policy Bug

Greetings List!
I believe I've found a bug in Mail::DomainKeys whereas an Internet
domain with a wildcard configured will return a CNAME when queried for a
_domainkey.domain.tld TXT DomainKey policy record. A non-TXT answer
possibility isn't handled properly in Mail::DomainKeys (a TXT answer is
expected for a TXT query... but that's too much to ask for from some
domain owners/servers) and results in "Use of uninitialized value in
split at /usr/lib/perl5/site_perl/5.8.5/Mail/DomainKeys/Policy.pm line
160" errors when SA tries to use this module.

You guys may or may not care; It appears that it has little to do with
SA, and is isolated to a small chunk of code in
Mail/DomainKeys/Policy.pm that handles the DNS answers. I didn't notice
this problem until I upgraded to Mail::DomainKeys 0.86 this morning.


I'm unsure how responsive the maintainer is; I've reported the issue
(and a proposed patch) below.

> -------- Original Message --------
> Subject:     Mail::DomainKeys::Policy Bug
> Date:     Tue, 10 Oct 2006 13:12:01 -0400
> From:     Ben Lentz <bl...@channing-bete.com>
> To:     anthonyu@cpan.org
>
>
>
> Hey Anthony,
> I was wondering if there might be a bug in Mail::DomainKeys 0.86. It 
would seem I'm getting a lot of:
>
> Use of uninitialized value in split at 
/usr/lib/perl5/site_perl/5.8.5/Mail/DomainKeys/Policy.pm line 160
>
> errors when I use the DomainKeys plugin for SpamAssassin. This seems 
to be happening for domains who have no explicit DK policy, and who also
employ a (stupid) wildcard redirect. For example, a spam that recently
came in from secureimport <dot> com was run though Mail::DomainKeys and
generated this error. Upon inspecting the _domainkey.secureimport <dot> com
record, I found that their servers answer _domainkey.secureimport <dot> 
com.
172629 IN  CNAME   www <dot> secureimport <dot> com when asked for
_domainkey.secureimport <dot> com TXT.
>
> For these types of misconfigurations, the fetch function in Policy.pm 
leaves $strn untouched because, although there are answers for the TXT
query (the first if test works), none of the answers are of the TXT type
(and $strn is left in the cold).
>
>        if (my $resp = $rslv->query($host, "TXT")) {
>                foreach my $ans ($resp->answer) {
>                        next unless $ans->type eq "TXT";
>                        $strn = join "", $ans->char_str_list or
>                                return;
>                }
>        } else {
>                # default policy is "sign some"
>                $strn = "o=~";
>        }
>
> ...later...
>
> sub parse_string {
>        my $text = shift;
>
> # $text is now empty, but split continues anyway and causes the "Use 
of uninitialized value" error.
>
> Attached is a proposed patch, that will change the next...unless to 
an if...else and will set $strn so long as the record resolves at all...
if the answer is not a TXT, it will at least default to "o=~". This may
not handle a case of multiple _domainkey.domain.tld records (e.g. a
CNAME and a TXT, for example, since the if...then will process for each
answer, with unpredictable results based on the order of the answers),
but such a case would be really, really out of spec, and can't be
expected to work... worst-case scenario, the return would be the default
"sign some".
>
> Thank you for your consideration, please let me know if you have any 
questions.
>



Re: Mail::DomainKeys::Policy Bug

Posted by Ben Lentz <bl...@channing-bete.com>.
This bug has been fixed in Mail::DomainKeys 0.88, which was released 
yesterday (10/12/2006).
http://search.cpan.org/src/ANTHONYU/Mail-DomainKeys-0.88/Changes

Again, not that this is an SA problem, but SA users with the DK plugin 
would've been likely to run into it.
> Greetings List!
> I believe I've found a bug in Mail::DomainKeys whereas an Internet
> domain with a wildcard configured will return a CNAME when queried for a
> _domainkey.domain.tld TXT DomainKey policy record. A non-TXT answer
> possibility isn't handled properly in Mail::DomainKeys (a TXT answer is
> expected for a TXT query... but that's too much to ask for from some
> domain owners/servers) and results in "Use of uninitialized value in
> split at /usr/lib/perl5/site_perl/5.8.5/Mail/DomainKeys/Policy.pm line
> 160" errors when SA tries to use this module.
>
> You guys may or may not care; It appears that it has little to do with
> SA, and is isolated to a small chunk of code in
> Mail/DomainKeys/Policy.pm that handles the DNS answers. I didn't notice
> this problem until I upgraded to Mail::DomainKeys 0.86 this morning.
>
>
> I'm unsure how responsive the maintainer is; I've reported the issue
> (and a proposed patch) below.
>
>> -------- Original Message --------
>> Subject:     Mail::DomainKeys::Policy Bug
>> Date:     Tue, 10 Oct 2006 13:12:01 -0400
>> From:     Ben Lentz <bl...@channing-bete.com>
>> To:     anthonyu@cpan.org
>>
>>
>>
>> Hey Anthony,
>> I was wondering if there might be a bug in Mail::DomainKeys 0.86. It 
> would seem I'm getting a lot of:
>>
>> Use of uninitialized value in split at 
> /usr/lib/perl5/site_perl/5.8.5/Mail/DomainKeys/Policy.pm line 160
>>
>> errors when I use the DomainKeys plugin for SpamAssassin. This seems 
> to be happening for domains who have no explicit DK policy, and who also
> employ a (stupid) wildcard redirect. For example, a spam that recently
> came in from secureimport <dot> com was run though Mail::DomainKeys and
> generated this error. Upon inspecting the _domainkey.secureimport 
> <dot> com
> record, I found that their servers answer _domainkey.secureimport 
> <dot> com.
> 172629 IN  CNAME   www <dot> secureimport <dot> com when asked for
> _domainkey.secureimport <dot> com TXT.
>>
>> For these types of misconfigurations, the fetch function in Policy.pm 
> leaves $strn untouched because, although there are answers for the TXT
> query (the first if test works), none of the answers are of the TXT type
> (and $strn is left in the cold).
>>
>>        if (my $resp = $rslv->query($host, "TXT")) {
>>                foreach my $ans ($resp->answer) {
>>                        next unless $ans->type eq "TXT";
>>                        $strn = join "", $ans->char_str_list or
>>                                return;
>>                }
>>        } else {
>>                # default policy is "sign some"
>>                $strn = "o=~";
>>        }
>>
>> ...later...
>>
>> sub parse_string {
>>        my $text = shift;
>>
>> # $text is now empty, but split continues anyway and causes the "Use 
> of uninitialized value" error.
>>
>> Attached is a proposed patch, that will change the next...unless to 
> an if...else and will set $strn so long as the record resolves at all...
> if the answer is not a TXT, it will at least default to "o=~". This may
> not handle a case of multiple _domainkey.domain.tld records (e.g. a
> CNAME and a TXT, for example, since the if...then will process for each
> answer, with unpredictable results based on the order of the answers),
> but such a case would be really, really out of spec, and can't be
> expected to work... worst-case scenario, the return would be the default
> "sign some".
>>
>> Thank you for your consideration, please let me know if you have any 
> questions.
>>
>
>
> ------------------------------------------------------------------------
>
> --- Mail/DomainKeys/Policy.pm.orig	2006-10-10 12:28:19.000000000 -0400
> +++ Mail/DomainKeys/Policy.pm	2006-10-10 14:01:38.000000000 -0400
> @@ -42,9 +42,13 @@
>  	
>  	if (my $resp = $rslv->query($host, "TXT")) {
>  		foreach my $ans ($resp->answer) {
> -			next unless $ans->type eq "TXT";
> -			$strn = join "", $ans->char_str_list or
> -				return;
> +			if ($ans->type eq "TXT") {
> +				$strn = join "", $ans->char_str_list or
> +					return;
> +			} else {
> +				# default policy is "sign some"
> +				$strn = "o=~";
> +			}
>  		}
>  	} else {
>  		# default policy is "sign some"
>