Skip to content
Permalink
Browse files

Validate: improve customer/employee name validation.

Brought to attention by
https://www.prestashop.com/forums/topic/981159-security-customer-validation-13-17/

Point here is, there are bots registering customer accounts with
their spam message as first name and their spam URL as last name.
This particular case should be stopped now.
  • Loading branch information...
Traumflug committed Apr 21, 2019
1 parent 956dc1a commit c733d5360d5292c12bb6a899748d9094bc608e05
Showing with 7 additions and 1 deletion.
  1. +7 −1 classes/Validate.php
@@ -240,10 +240,16 @@ public static function isImageSize($size)
*
* @since 1.0.0
* @version 1.0.0 Initial version
* @version 1.1.0 Don't accept 'http', 'www' or '/', do accept ','.
*/
public static function isName($name)
{
return (bool) preg_match(Tools::cleanNonUnicodeSupport('/^[^0-9!<>,;?()@"°{}_$%:]*$/u'), stripslashes($name));
$name = stripslashes($name);
return ! preg_match('/www|http/ui', $name)
&& preg_match(
Tools::cleanNonUnicodeSupport('/^[^0-9\/!<>;?()@"°{}_$%:]*$/u'),
$name
);
}
/**

8 comments on commit c733d53

@Eolia

This comment has been minimized.

Copy link
Contributor

replied Apr 21, 2019

Delete this line please:
$name = stripslashes($name);
It's a non-sense^^

@doekia

This comment has been minimized.

Copy link
Contributor

replied Apr 21, 2019

May I ask why you insist preserving the / ?
worste this scenario let me enter a path /var/www/site/hack.php and you consider it a valid name

@Traumflug

This comment has been minimized.

Copy link
Contributor Author

replied Apr 21, 2019

Delete this line please:
$name = stripslashes($name);

Backslashes in names are probably not a good idea. This could easily open attack vectors we're not aware of, yet.

@Traumflug

This comment has been minimized.

Copy link
Contributor Author

replied Apr 21, 2019

May I ask why you insist preserving the / ?

Unless I'm mistaken, they're not preserved. It's the \/ right after 0-9.

grafik

That said, I was indeed not aware that this long regex changed as well. So I made another commit with this, except for the comma ("George Bush, Jr."): 7ad147d

@Eolia

This comment has been minimized.

Copy link
Contributor

replied Apr 21, 2019

Backslashes in names are probably not a good idea. This could easily open attack vectors we're not aware of, yet.

Waoouh !!!
This function not sanitizes but controls, so if you delete backslashes, you control what ????

@Traumflug

This comment has been minimized.

Copy link
Contributor Author

replied Apr 21, 2019

Waoouh

This is thirty bees. Here we have no need to freak out :-)

@Eolia

This comment has been minimized.

Copy link
Contributor

replied Apr 21, 2019

I don't panic but I think you reason like Prestashop :-/

@doekia

This comment has been minimized.

Copy link
Contributor

replied Apr 22, 2019

The screenshot displays invalid because www not because /
Try /etc/hosts it does not trigger invalid
If we stripslashes befor testing if there is any, there is none. QED

Please sign in to comment.
You can’t perform that action at this time.