svn commit: r1935671 - in spamassassin/trunk: lib/Mail/SpamAssassin/Plugin t

Author: fkento
Date: Sat Jun 27 01:14:05 2026
New Revision: 1935671

Log:
Redirectors: only extract embedded URIs that actually look like URLs

_extract_embedded_uri turned any url_redirector_params value into an
embedded URI, prepending http:// to bare tokens. Referral codes and
labels such as r=to8ex or redirect=app-store-no-desktop became bogus
http://to8ex / http://app-store-no-desktop URIs that were added to the
URI detail list and subjected to URIBL/HASHBL lookups, causing false
positives.

Require the captured value to look like a URL (explicit/encoded scheme,
scheme-relative //host, or bare host.tld/path) before accepting it. Also
guard against a user-supplied param regex with no capture group, which
left $1 undefined and fabricated a bare "http://".

Add regression tests covering both.

Modified:
   spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/Redirectors.pm
   spamassassin/trunk/t/redirectors_match.t

Modified: spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/Redirectors.pm
==============================================================================
--- spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/Redirectors.pm	Sat Jun 27 00:45:04 2026	(r1935670)
+++ spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/Redirectors.pm	Sat Jun 27 01:14:05 2026	(r1935671)
@@ -1041,6 +1041,18 @@ sub _extract_embedded_uri {
   local($1);
   if (($rest =~ /(?:\?|\&)$rreg/gis) || ($rest =~ /(?:\/|\_|\=)((?:https?:)?\/\/.*)/)) {
     my $newuri = $1;
+    # A user-supplied url_redirector_params with no capture group leaves $1
+    # undefined here; without this guard we would fabricate a bare "http://".
+    return unless defined $newuri;
+    # The param value is only an embedded URI if it actually looks like one:
+    # an explicit/encoded scheme, a scheme-relative //host, or a bare
+    # host.tld/path. A bare token (referral code, label, etc.) is not a URI
+    # and must not be turned into a fabricated http://<token>.
+    unless ($newuri =~ m{^https?(?::|%3a)}i
+         || $newuri =~ m{^//}
+         || $newuri =~ m{^[^/?#\s]+\.[^/?#\s]+/}) {
+      return;
+    }
     dbg("Found embedded uri $newuri in $uri");
     $newuri = 'http://' . $newuri if $newuri !~ /^http/;
     return $newuri;

Modified: spamassassin/trunk/t/redirectors_match.t
==============================================================================
--- spamassassin/trunk/t/redirectors_match.t	Sat Jun 27 00:45:04 2026	(r1935670)
+++ spamassassin/trunk/t/redirectors_match.t	Sat Jun 27 01:14:05 2026	(r1935671)
@@ -63,8 +63,28 @@ my @cases = (
   [ 'https://app.spa.example/foo',   'selenium', 'selenium method on leading-dot subdomain' ],
 );
 
-# clear_url_redirector tests add 4 more checks
-plan tests => scalar(@cases) + 4;
+# A param value is only an embedded URI when it actually looks like one. A bare
+# token (referral code, label) must NOT be turned into a fabricated http://<token>.
+# Use a param list that includes the short params (r, redirect) that produced the
+# original Substack false positives, not just url/u.
+my $embedded_conf =
+  { url_redirector_params => qr/(?:url|u|r|redir|redirect)=(.*)/i };
+my @embedded = (
+  # [ uri, expected_extraction (undef = no embedded uri), desc ]
+  [ 'https://safe.example/?url=https://evil.com/path', 'https://evil.com/path',
+    'explicit scheme is extracted' ],
+  [ 'https://safe.example/?url=evil.com/landing', 'http://evil.com/landing',
+    'bare host.tld/path is extracted' ],
+  [ 'https://safe.example/?url=//evil.com/path', 'http:////evil.com/path',
+    'scheme-relative //host is extracted' ],
+  [ 'https://substack.com/signup?foo=bar&r=to8ex', undef,
+    'bare referral token (r=) is NOT an embedded uri' ],
+  [ 'https://open.substack.com/notes?utm_campaign=open-in-app&redirect=app-store-no-desktop', undef,
+    'bare hyphenated label (redirect=) is NOT an embedded uri' ],
+);
+
+# +1 no-capture-group check, +4 clear_url_redirector checks
+plan tests => scalar(@cases) + scalar(@embedded) + 1 + 4;
 
 for my $c (@cases) {
   my ($uri, $expect_method, $desc) = @$c;
@@ -77,6 +97,25 @@ for my $c (@cases) {
   }
 }
 
+for my $c (@embedded) {
+  my ($uri, $expect, $desc) = @$c;
+  my $r = Mail::SpamAssassin::Plugin::Redirectors::_extract_embedded_uri($uri, $embedded_conf);
+  if (defined $expect) {
+    is($r, $expect, $desc);
+  } else {
+    ok(!defined $r, $desc);
+  }
+}
+
+# A url_redirector_params regex with no capture group leaves $1 undefined; the
+# extractor must bail rather than fabricate a bare "http://".
+{
+  my $nocap_conf = { url_redirector_params => qr/(?:r|redirect)=[^&]+/i };
+  my $r = Mail::SpamAssassin::Plugin::Redirectors::_extract_embedded_uri(
+    'https://substack.com/signup?foo=bar&r=to8ex', $nocap_conf);
+  ok(!defined $r, 'no-capture-group param regex yields no embedded uri');
+}
+
 # clear_url_redirector behavior
 {
   my $conf = { url_redirector_params => qr/(?:url|u)=(.*)/i };