Bug 5878 - IPV4_ADDRESS regexp matches ip.ad.dr.in-addr.arpa format
Summary: IPV4_ADDRESS regexp matches ip.ad.dr.in-addr.arpa format
Status: RESOLVED FIXED
Alias: None
Product: Spamassassin
Classification: Unclassified
Component: Libraries (show other bugs)
Version: 3.2.4
Hardware: All All
: P4 normal
Target Milestone: 3.3.0
Assignee: SpamAssassin Developer Mailing List
URL:
Whiteboard:
Keywords:
: 6256 (view as bug list)
Depends on:
Blocks:
 
Reported: 2008-04-06 20:12 UTC by Yusuf Goolamabbas
Modified: 2010-02-01 11:26 UTC (History)
3 users (show)


Attachment Type Modified Status Actions Submitter/CLA Status
Correct ip address boundary check patch None Tony Finch [HasCLA]
Correct ip address boundary check patch None Tony Finch [HasCLA]
Suggested patch patch None Mark Martinec [HasCLA]
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuf Goolamabbas 2008-04-06 20:12:58 UTC 
Hi, via some recent testing. I saw that RCVD_NUMERIC_HELO was being fired when the   helo detected by SA was of the form a.b.c.d.in-adddr.arpa (where a.b.c.d) was an ip address in reverse octect form

I wrote the following test case which uses the constant IPV4_ADDRESS from the SA source to show this behaviour. 

#!/usr/bin/perl

use strict;
use warnings;
use Test::More tests => 1;

use constant IPV4_ADDRESS => qr/\b
                    (?:1\d\d|2[0-4]\d|25[0-5]|\d\d|\d)\.
                    (?:1\d\d|2[0-4]\d|25[0-5]|\d\d|\d)\.
                    (?:1\d\d|2[0-4]\d|25[0-5]|\d\d|\d)\.
                    (?:1\d\d|2[0-4]\d|25[0-5]|\d\d|\d)
                  \b/ox;

sub check_ipv4 {
   my ($rcvd) = @_ ;
   my $IP_ADDRESS = IPV4_ADDRESS;
   if ($rcvd =~ /helo=($IP_ADDRESS)\b/i) {
     return 1;
   }
   return 0;
}

my $helostring = "helo=242.8.168.192.in-addr.arpa";

ok(check_ipv4($helostring) == 1);
Comment 1 Tony Finch 2008-10-27 05:38:54 UTC 
Created attachment 4383 [details]
Correct ip address boundary check
Comment 2 Tony Finch 2008-10-27 05:39:13 UTC 
Created attachment 4384 [details]
Correct ip address boundary check
Comment 3 Tony Finch 2008-10-27 05:40:24 UTC 
My site has encountered a false positive because of this bug.
(Aside: I am not sure if bug#5767 is related or not - it is not reproducible.)

I don't think the problem here is IPV4_ADDRESS, but rather the way that check_for_numeric_helo() uses it. The code uses $rcvd =~ /helo=($IP_ADDRESS)\b/i but \b matches dots inside hostnames not the ends of hostnames. It should be a space instead.

Patch attached.
Comment 4 dan pritts 2008-12-12 13:34:36 UTC 
I got a bunch of similar FP's with RCVD_NUMERIC_HELO today; a bunch of my staff was staying at the same hotel, and using the hotel's internet service.

[30294] dbg: rules: ran header rule HELO_DYNAMIC_IPADDR2 ======> got hit: 
"[ ip=66.238.164.2 rdns=66.238.164.2.ptr.us.xo.net helo=154.3.168.192.in-addr.arpa.noptr.antlabs.com 
by=magus.merit.edu ident= envfrom= intl=0 id=ADD7B225CEB auth= "

The actual Received line is:

Received: from 154.3.168.192.in-addr.arpa.noptr.antlabs.com (66.238.164.2.ptr.us.xo.net [66.238.164.2])
        by magus.merit.edu (Postfix) with ESMTP id ADD7B225CEB;
        Thu, 11 Dec 2008 22:40:02 -0500 (EST)

Tony's analysis seems correct to me.  I'm not sure if there's something better than using a literal space in the regexp (\s instead perhaps?) but \b is clearly wrong.

thanks all for all your hard work on SA.
Comment 5 Mark Martinec 2009-08-06 12:33:12 UTC 
If it is still broken, it should be fixed for 3.3.0.
Comment 6 Mark Martinec 2009-08-13 08:54:47 UTC 
Created attachment 4516 [details]
Suggested patch
Comment 7 Mark Martinec 2009-08-13 09:08:22 UTC 
  Bug 5878: IPV4_ADDRESS regexp matches ip.ad.dr.in-addr.arpa format
  (attempting a fix; do we have any tests for this?)
Sending        lib/Mail/SpamAssassin/Plugin/RelayEval.pm
Committed revision 803940.
Comment 8 Justin Mason 2009-08-13 09:45:43 UTC 
(In reply to comment #7)
>   Bug 5878: IPV4_ADDRESS regexp matches ip.ad.dr.in-addr.arpa format
>   (attempting a fix; do we have any tests for this?)

yep, I think we do.  t/ip_addrs.t
Comment 9 Mark Martinec 2009-08-13 11:25:03 UTC 
closing
Comment 10 Adam Katz 2010-02-01 11:26:07 UTC 
*** Bug 6256 has been marked as a duplicate of this bug. ***