search

spamhaus / spamassassin-dqs

Spamhaus code for the Spamassassin plugin. See https://docs.spamhaustech.com/40-real-world-usage/SpamAssassin/000-intro.html
Apache License 2.0
54 stars 16 forks source link

Use of uninitialized value $this_domain #10

Closed orlitzky closed 5 years ago

orlitzky commented 5 years ago

When receiving an email from a malformed address, the following can be logged:

Jul 7 01:22:58 mx1 amavis[2269]: (02269-07) _WARN: Use of uninitialized value $this_domain in concatenation (.) or string at /usr/lib64/perl5/vendor_perl/5.28.2/Mail/SpamAssassin/Plugin/SH.pm line 172.

Jul 7 01:22:58 mx1 amavis[2269]: (02269-07) _WARN: Use of uninitialized value $this_domain in hash element at /usr/lib64/perl5/vendor_perl/5.28.2/Mail/SpamAssassin/Plugin/SH.pm line 173.

Jul 7 01:22:58 mx1 amavis[2269]: (02269-07) _WARN: Use of uninitialized value $this_domain in hash element at /usr/lib64/perl5/vendor_perl/5.28.2/Mail/SpamAssassin/Plugin/SH.pm line 174.

Jul 7 01:22:58 mx1 amavis[2269]: (02269-07) _WARN: Use of uninitialized value $this_domain in hash element at /usr/lib64/perl5/vendor_perl/5.28.2/Mail/SpamAssassin/Plugin/SH.pm line 196.

Jul 7 01:22:58 mx1 amavis[2269]: (02269-07) _WARN: Use of uninitialized value $this_domain in concatenation (.) or string at /usr/lib64/perl5/vendor_perl/5.28.2/Mail/SpamAssassin/Plugin/SH.pm line 197.

These lead to,

Jul 7 01:22:58 mx1 amavis[2269]: (02269-07) _WARN: dns: new_dns_packet (domain=..zrd.dq.spamhaus.net. type=A class=IN) failed: a domain name contains a null label

Now, this was a message we probably shouldn't have accepted in the first place: the sender's address was a bare hostname (e.g. foo.bar.invalid). However, it was to our postmaster address, and we're pretty lenient with what we'll accept to postmaster, so that people can effectively complain if I screw things up.

From looking at the code, most other places where $this_domain is set first check that the string is an email address (using a regex) before splitting on the @ character to obtain the domain. The sub _get_headers_domains, on the other hand... does some other stuff that I don't understand. I have a copy of the email that triggered the warnings, though, and the "From:" header contains the bare hostname, e.g.

From: foo.bar.invalid

Since that subroutine looks at the "From:" header, he's my primary suspect.

ricalfieri commented 5 years ago

I pushed a fix for checking empty variables, please pull the new SH.pm and let me know

orlitzky commented 5 years ago

Looks good, thank you!