[Fix] url_suspect: drop dead branch in user field check

Both arms of the length_thresholds.suspicious branch inserted the
very same URL_USER_PASSWORD finding, so the threshold never affected
anything. Remove the branch together with the now unused setting.

Reject mailto URLs explicitly in the same check: every email address
carries a local part, so the presence of a user is meaningless there.
The mailto parser does not set has_user today, but that intent belongs
in the check rather than in a parser detail that may change.

Cover both cases functionally: a user field without a password must
still fire, email addresses must stay silent.

diff --git a/src/plugins/lua/url_suspect.lua b/src/plugins/lua/url_suspect.lua
index ecad86ceb4762091d9f0fa3cf32658d45e0315c6..600536b4f9c4ee29fd86df14ccb0eade9e237c73 100644 (file)
--- a/src/plugins/lua/url_suspect.lua
+++ b/src/plugins/lua/url_suspect.lua
@@ -71,7 +71,6 @@ local settings = {
     user_password = {
       enabled = true,
       length_thresholds = {
-        suspicious = 64,
         long = 128,
         very_long = 256
       }
@@ -272,6 +271,14 @@ function checks.user_password_analysis(task, url, cfg)
   local url_flags_tab = rspamd_url.flags
   local flags = url:get_flags_num()
 
+  -- Emails always carry a user part (the local part of the address), so this
+  -- check is meaningless for them: it would fire on every address in a
+  -- message. The mailto parser does not set has_user, but do not rely on that
+  -- alone - reject mailto explicitly.
+  if url:get_protocol() == 'mailto' then
+    return findings
+  end
+
   -- Check if user field present
   if bit.band(flags, url_flags_tab.has_user) == 0 then
     return findings
@@ -286,7 +293,9 @@ function checks.user_password_analysis(task, url, cfg)
 
   lua_util.debugm(N, task, "Checking user field length: %d chars", user_len)
 
-  -- Length-based detection (get host only when needed for options)
+  -- Escalating severity, one symbol per URL: an oversized user field is a
+  -- stronger signal than the mere presence of one. Whether a password follows
+  -- the user is irrelevant here (and the parser does not keep it anyway).
   if user_len > cfg.length_thresholds.very_long then
     table.insert(findings, {
       symbol = symbols.user_very_long,
@@ -298,20 +307,12 @@ function checks.user_password_analysis(task, url, cfg)
       options = { string.format("%d", user_len) }
     })
   else
-    -- Get host only for these cases where we need it in options
+    -- Get host only for this case where we need it in options
     local host = url:get_host()
-    if user_len > cfg.length_thresholds.suspicious then
-      table.insert(findings, {
-        symbol = symbols.user_password,
-        options = { host or "unknown" }
-      })
-    else
-      -- Normal length user
-      table.insert(findings, {
-        symbol = symbols.user_password,
-        options = { host or "unknown" }
-      })
-    end
+    table.insert(findings, {
+      symbol = symbols.user_password,
+      options = { host or "unknown" }
+    })
   end
 
   -- Optional: check pattern map if configured

diff --git a/test/functional/cases/001_merged/400_url_suspect.robot b/test/functional/cases/001_merged/400_url_suspect.robot
index 52c7efae0574ba30fe417246feb05fde23908792..cf404e840ef4f2c10e779704b9f6a0d3ac5daa4b 100644 (file)
--- a/test/functional/cases/001_merged/400_url_suspect.robot
+++ b/test/functional/cases/001_merged/400_url_suspect.robot
@@ -18,6 +18,22 @@ URL Suspect - Very Long User Field
   ...  Settings={symbols_enabled = [URL_SUSPECT_CHECK, URL_USER_LONG, URL_USER_VERY_LONG, URL_USER_PASSWORD]}
   Expect Symbol With Exact Options  URL_USER_VERY_LONG  300
 
+URL Suspect - User Without Password
+  # A user part with no password is still a credential-shaped URL and must fire
+  Scan File  ${RSPAMD_TESTDIR}/messages/url_suspect_user_no_password.eml
+  ...  Settings={symbols_enabled = [URL_SUSPECT_CHECK, URL_USER_LONG, URL_USER_VERY_LONG, URL_USER_PASSWORD]}
+  Expect Symbol With Exact Options  URL_USER_PASSWORD  phishing.com
+  Do Not Expect Symbol  URL_USER_LONG
+  Do Not Expect Symbol  URL_USER_VERY_LONG
+
+URL Suspect - Email Addresses Are Not URL Users
+  # Every email address has a local part; it must never trigger the user check
+  Scan File  ${RSPAMD_TESTDIR}/messages/url_suspect_email_user.eml
+  ...  Settings={symbols_enabled = [URL_SUSPECT_CHECK, URL_USER_LONG, URL_USER_VERY_LONG, URL_USER_PASSWORD]}
+  Do Not Expect Symbol  URL_USER_PASSWORD
+  Do Not Expect Symbol  URL_USER_LONG
+  Do Not Expect Symbol  URL_USER_VERY_LONG
+
 URL Suspect - Numeric IP
   # Test numeric IP detection
   Scan File  ${RSPAMD_TESTDIR}/messages/url_suspect_numeric_ip.eml

diff --git a/test/functional/messages/url_suspect_email_user.eml b/test/functional/messages/url_suspect_email_user.eml
new file mode 100644 (file)
index 0000000..32b70e1
--- /dev/null
+++ b/test/functional/messages/url_suspect_email_user.eml
@@ -0,0 +1,11 @@
+From: sender@example.com
+To: victim@example.com
+Subject: Test email addresses must not be treated as user in URL
+Content-Type: text/html; charset=utf-8
+
+<html>
+<body>
+<p>Write to us at support@example.com or use the link below:</p>
+<a href="mailto:sales@example.com">Contact sales</a>
+</body>
+</html>

diff --git a/test/functional/messages/url_suspect_user_no_password.eml b/test/functional/messages/url_suspect_user_no_password.eml
new file mode 100644 (file)
index 0000000..427aea7
--- /dev/null
+++ b/test/functional/messages/url_suspect_user_no_password.eml
@@ -0,0 +1,11 @@
+From: sender@example.com
+To: victim@example.com
+Subject: Test URL with user and no password
+Content-Type: text/html; charset=utf-8
+
+<html>
+<body>
+<p>Click this link:</p>
+<a href="http://user@phishing.com/page">Click Here</a>
+</body>
+</html>