-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Block Usage of User Registration emails for phishing/malware spam #20142
Conversation
… maintain backwards compatability Fixes issue joomla#18203
language/en-GB/en-GB.lib_joomla.ini
Outdated
@@ -225,6 +225,7 @@ JLIB_DATABASE_ERROR_USERNAME_CANNOT_CHANGE="Can't use this username." | |||
JLIB_DATABASE_ERROR_USERNAME_INUSE="Username in use." | |||
JLIB_DATABASE_ERROR_VALID_AZ09="Please enter a valid username. No space at beginning or end, at least %d characters and must <strong>not</strong> have the following characters: < > \ " ' % ; ( ) &." | |||
JLIB_DATABASE_ERROR_VALID_MAIL="Please enter a valid email address." | |||
JLIB_DATABASE_ERROR_USERNAME_NAME_MUST_NOT_CONTAIN_LINKS="Username and Name fields must not contain links" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sort key in alpha order.
libraries/src/Table/User.php
Outdated
// Block using registration emails to send spam/malware/phishing emails with embedded links | ||
if (preg_match('#https?#', $this->username) || preg_match('#https?#', $this->name)) | ||
{ | ||
$this->setError(\JText::sprintf('JLIB_DATABASE_ERROR_USERNAME_NAME_MUST_NOT_CONTAIN_LINKS', 2)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to use sprintf since string does not have a placeholder.
Not acceptable. On To be honest, any sort of "dodgy email" filter is going to be very arbitrary and I'm not sure what measures core should actually take about this, if any at all, because it is going to result in an arbitrary decision to disallow some kind of username syntax for reasons that equate to nothing more than "if someone gets emailed with a username in that email content it will look like spam to the human eye". |
This is a valid issue. We have an open issue on the tracker regarding this already. However just preventing links doesn't go far enough for me and its only a partial solution. All you really need to do is to have a more realistic limit on the max number of characters in a name/username |
libraries/src/Table/User.php
Outdated
@@ -195,7 +195,15 @@ public function check() | |||
|
|||
return false; | |||
} | |||
|
|||
// Block using registration emails to send spam/malware/phishing emails with embedded links | |||
if (preg_match('#https?#', $this->username) || preg_match('#https?#', $this->name)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that it's a good solution to block strings in usernames or names because "http" or "https" are not reliable identifiers for links and because usernames like "httpMaster" or similiar shouldn't be blocked. Spammers will use several combinations like HtTp. You would have to block them all (and lots of other protocols).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about using
https?:\/\/
in the preg_match ??
I didn't spot this 'issue' myself - I came in to work yesterday to find 900+ new user registrations all sending out this type of email. So it is a real issue that could at the very least affect the email server reputation of a Joomla site. In my case I didn't have captcha enabled on user registration for this particular site which is why the number got so big. Limiting the length of username and name would help but the limit would need to be pretty short when URL shorteners are introduced into the equation. I suppose blocking back slashes could help to limit the use of most URL shorteners. I see the issue with banning the "period" for usernames - and I suspect there are real "names" with periods in them. |
I get it's real and annoying. I just don't know how you do a spam rule in front of a username that doesn't either come across as a "we're trying to manually filter spam" type thing (which becomes a high maintenance burden because such a rule would have to commonly updated or you'd need to ensure registration triggers plugin events before saving so that the username could be pushed through a spam detection/filtering service the same way comment or forum post messages are and registration blocked based on that) or impose restrictions on legitimate use cases. |
Personally I think the name is more important to deal with than the username since it is used right at the start of the email. UsersModelRegistration::getData trigger onContentPrepareData with a suitable context set (com_users.registration) so a solution could be implemented as a plugin. But I wonder if there is a 'responsibility' on the core code to make this type of exploitation of mail server of a Joomla site less likely - even if it can't be blocked completely?? p.s. As a wild, out there, idea we could replace the period with a 'one dot leader' in the text of the email (http://www.fileformat.info/info/unicode/char/2024/index.htm) which would make any embedded domains unclickable - but that probably opens up a whole can of worms |
I still don't know. Any filter/validation rule on a username or display/real name is arbitrary at best, doing it in direct defense against potential spam attacks makes it harder to build/maintain in any way short of "outsource to plugins that can hook to spam filter to address this". Having a username filter is easy (usernames can match regex). A display/real name filter isn't. |
Simply use \b (word boundary)
Use similar detection
And yes it will block names / usernames containing word starting with www. #(\b\/\/b|\swww.|^www.)#i |
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/20142. |
Updated regex check on username to block :// (as in http:// https:// ftp:// etc.) www. and ftp. in the username/name
I have updated the regex to this
since
wasn't matching http:// etc. The primary focus of this PR was not to reduce Joomla being used to generate spam - it was to reduce the risk of it being used for phishing or distribution of malware site links |
libraries/src/Table/User.php
Outdated
@@ -195,7 +195,16 @@ public function check() | |||
|
|||
return false; | |||
} | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove tabs
libraries/src/Table/User.php
Outdated
|
||
return false; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove tabs
language/en-GB/en-GB.lib_joomla.ini
Outdated
@@ -223,6 +223,7 @@ JLIB_DATABASE_ERROR_USERGROUP_TITLE_EXISTS="User group title already exists. Tit | |||
JLIB_DATABASE_ERROR_USERLEVEL_NAME_EXISTS="Level with the name "%s" already exists." | |||
JLIB_DATABASE_ERROR_USERNAME_CANNOT_CHANGE="Can't use this username." | |||
JLIB_DATABASE_ERROR_USERNAME_INUSE="Username in use." | |||
JLIB_DATABASE_ERROR_USERNAME_NAME_MUST_NOT_CONTAIN_LINKS="Username and Name fields must not contain links" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove fields
. Add a period.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please update the language string with the comment from @Quy
Changed made as requested. Since I created this PR at least half the client sites I have looked at have been used by Russian scammers taking advantage of this 'loop hole' in Joomla - with thousands of potential victims hit. I know this isn't a perfect and final solution but it is an essential first step in my opinion |
any modification in lib.joomla.ini has to be done both in admin and site files |
@infograf768 backend language file modified to replicate frontend change. |
Is it something that should not be arbitrary and have a possible control in User Manager options (Disable in Otherwise there is a bunch of extensions that allows this, for example https://extensions.joomla.org/extension/restrict-usernames/ Other option is simply to restrict by max characters as already mentioned. |
Sorry that it took so long to respond. I would like to see it as options where people can specify rules for usernames. As others were indicating, this is also already doable with extensions. So I'm closing it for now. Thanks for your help making Joomla better. |
Pull Request for Issue # .
Summary of Changes
Add new check to com_users User Table check on data validity to stop Joomla from being used to distribute phishing/malware emails.
Testing Instructions
In unaltered Joomla site make sure that user registrations are enabled and in the frontend create a new user with data similar to the following (use plain text not HTML tags for the links)
name: Fred Flintstone. You have been selected to win $1000 - simply download your claim form from https://www.dodgywebsite.com/dubiousfile.html
username:fred. You have been selected to win $1000 - simply download your claim form from https://www.dodgywebsite.com/dubiousfile.html
Then fill the rest of the form with valid data.
Expected result
If you do not apply this patch the email address used in the form gets an email that tempts them to click the link in the message body - or your Joomla site could get flagged as a distributor of malware. Note that some email servers may block the sending of this type of message but most will not.
This blatant attempt to send malware using Joomla should be blocked
Actual result
Dodgy email is sent!
Apply the patch and the username and name fields are blocked from including http:// or https:// - unfortunately this still doesn't block names/usernames containing valid URLs which many email packages will still render as clickable links :(
The only way I can see to block this is to stop usernames and names from including a full stop/period. Not sure of the implications of that - probably ok if we only apply the restriction to new accounts
e.g.
Documentation Changes Required