Skip to content

Conversation

@meetvora69
Copy link

Description (*)

First name and Last name field must not allow to enter URL and Special character's

Fixed Issues (if relevant)

  1. First name and Last name field must not allow to enter URL and Special character's #23487: First name and Last name field must not allow to enter URL and Special character's

Manual testing scenarios (*)

  1. Go to Admin
  2. Customers > All customers > Add New Customer
  3. Enter URL details for First name Last name field

Questions or comments

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds are green)

@m2-assistant
Copy link

m2-assistant bot commented Jun 29, 2019

Hi @meetvora69. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

  • @magento give me test instance - deploy test instance based on PR changes
  • @magento give me 2.3-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

Copy link
Contributor

@ihor-sviziev ihor-sviziev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @meetvora69,

I just reviewed your fix and found one issue - people will not be able to put non English name & surname. For instance you can't put there "Müller", or "Иван" anymore.
So there should be some blacklist of special symbols.

Could you double check your solution?

@ihor-sviziev
Copy link
Contributor

Hi @meetvora69,
Thank you for your contribution. Unfortunately we didn’t get response from you after more than 14 days, so I’m closing this PR.
Feel free to fix issues listed in my last review and reopen PR.

@m2-assistant
Copy link

m2-assistant bot commented Jul 27, 2019

Hi @meetvora69, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

@cbichis
Copy link

cbichis commented May 14, 2020

This is the most frequent attack nowdays, we need to have a way to filter out URL's...

@ihor-sviziev
Copy link
Contributor

ihor-sviziev commented May 14, 2020

Hi @cbichis,
Original issue was closed as not an issue. More details with explanation why, you could find there #23487 (comment).
Could you explain what attack do you mean?

@cbichis
Copy link

cbichis commented May 14, 2020

The spam bots are registering or signing up to the newsletter and they are using on first name/last name URL's. This would cause the registration confirmation email to include malicious URL's which of course are dangerous for anyone who clicks on them.

With a enough large botnet it would be simple to send thousands of such emails. You can say ther are other security measures possible for such issues, but heh, by default I think Magento should ship basic protection for forms, which won't depend on user configuration (as it's captchas by example).

Maybe the special characters are not always an issue but stripping URL's from forms is quite a must have...

@ihor-sviziev
Copy link
Contributor

@cbichis oh, I see. Seems like following issue contains all needed info and should be confirmed #28041

@cbichis
Copy link

cbichis commented May 15, 2020

Yeah, but thats a bit a diff topic.

I am pointing out here that it's quite hard to understand Magento with the default settings to have such security holes... For the custom made apps I am doing I also strip out not just tags but also URL's from the fields...

@ihor-sviziev
Copy link
Contributor

@cbichis, I see. Seems like this is duplicate of #7266 #28041.

Looks like issue is valid, should be fixable by enabling recaptcha and adding some WAF.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants