From: Vsevolod Stakhov Date: Sat, 25 Jul 2026 14:05:36 +0000 (+0100) Subject: [Fix] url_suspect: drop dead branch in user field check X-Git-Tag: 4.1.3~6 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b1d5cbec6dac6687725d734f3c93037cf4eb104a;p=thirdparty%2Frspamd.git [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 ecad86ceb4..600536b4f9 100644 --- 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 52c7efae05..cf404e840e 100644 --- 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 index 0000000000..32b70e1b54 --- /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 + + + +

Write to us at support@example.com or use the link below:

+Contact sales + + 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 index 0000000000..427aea7305 --- /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 + + + +

Click this link:

+Click Here + +