Bug 7725 - Perl taint bug with URIDNSBL netmask calculations
Summary: Perl taint bug with URIDNSBL netmask calculations
Status: RESOLVED FIXED
Alias: None
Product: Spamassassin
Classification: Unclassified
Component: Plugins (show other bugs)
Version: 3.4.2
Hardware: All All
: P2 major
Target Milestone: 3.4.3
Assignee: SpamAssassin Developer Mailing List
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-06-20 23:52 UTC by Henrik Krohns
Modified: 2019-06-21 08:35 UTC (History)
2 users (show)


Attachment Type Modified Status Actions Submitter/CLA Status
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 Henrik Krohns 2019-06-20 23:52:13 UTC 
While trying to activate taint for all tests, I encountered a baffling perl bug, dnsbl_subtests.t stopped working with 5.14.0.

With some values debugging this was seen

taint on:
 n1 2130706690 delim / n2 4294967295 rdatanum 2130706690
 n1 & n2: 0010106290

taint off:
 n1 2130706690 delim / n2 4294967295 rdatanum 2130706690
 n1 & n2: 2130706690

Soon after I found out that

"2130706690" & "4294967295" = 0010106290
2130706690 & 4294967295 = 2130706690

When tainted, $n1 $n2 are considered strings and not ints..

I tried Perl 5.22 which did not have this bug.

Simple fix is forcing int($n1) and all works fine again. Please vote to commit for 3.4.3.

--- lib/Mail/SpamAssassin/Plugin/URIDNSBL.pm    (revision 1861709)
+++ lib/Mail/SpamAssassin/Plugin/URIDNSBL.pm    (working copy)
@@ -1110,8 +1110,8 @@
         !defined $n2  ? ($rdatanum & $n1) &&                  # mask only
                           (($rdatanum & 0xff000000) == 0x7f000000)  # 127/8
       : $delim eq '-' ? $rdatanum >= $n1 && $rdatanum <= $n2  # range
-      : $delim eq '/' ? ($rdatanum & $n2) == ($n1 & $n2)      # value/mask
-      : 0;
+      : $delim eq '/' ? ($rdatanum & $n2) == (int($n1) & $n2) # value/mask
+      : 0; # notice int($n1) instead of $n1 to fix perl ~5.14 taint bug

       dbg("uridnsbl: %s . %s -> %s, %s, %08x %s %s",
           $ent->{domain}, $ent->{zone}, $rdatastr, $rulename, $rdatanum,
Comment 1 Henrik Krohns 2019-06-20 23:54:40 UTC 
To clarify, this affected all /slash notations, which are not used in default ruleset.

urirhssub FOO bar A 1.2.3.4/255.255.255.0
Comment 2 Giovanni Bechis 2019-06-21 07:14:03 UTC 
makes sense to fix it this way
+1
Comment 3 Henrik Krohns 2019-06-21 08:35:40 UTC 
Banging this out of the way

Sending        spamassassin-3.4/lib/Mail/SpamAssassin/Plugin/URIDNSBL.pm
Sending        trunk/lib/Mail/SpamAssassin/Plugin/URIDNSBL.pm
Transmitting file data ..done
Committing transaction...
Committed revision 1861762.
Comment 4 Henrik Krohns 2019-06-21 08:35:49 UTC 
Banging this out of the way

Sending        spamassassin-3.4/lib/Mail/SpamAssassin/Plugin/URIDNSBL.pm
Sending        trunk/lib/Mail/SpamAssassin/Plugin/URIDNSBL.pm
Transmitting file data ..done
Committing transaction...
Committed revision 1861762.