Enhanced Customer Data Validation to Mitigate Code Injection Risks

app/code/Magento/Contact/Model/Mail.php

@@ -10,6 +10,9 @@
 use Magento\Store\Model\StoreManagerInterface;
 use Magento\Framework\App\ObjectManager;
 use Magento\Framework\App\Area;
+use Magento\Framework\Validator\GlobalForbiddenPatterns;
+use Magento\Framework\App\Config\ScopeConfigInterface;
+use Magento\Framework\Exception\LocalizedException;
 
 class Mail implements MailInterface
 {
@@ -33,24 +36,32 @@ class Mail implements MailInterface
      */
     private $storeManager;
 
+    /**
+     * @var GlobalForbiddenPatterns
+     */
+    private $forbiddenPatternsValidator;
+
     /**
      * Initialize dependencies.
      *
      * @param ConfigInterface $contactsConfig
      * @param TransportBuilder $transportBuilder
      * @param StateInterface $inlineTranslation
      * @param StoreManagerInterface|null $storeManager
+     * @param GlobalForbiddenPatterns $forbiddenPatternsValidator
      */
     public function __construct(
         ConfigInterface $contactsConfig,
         TransportBuilder $transportBuilder,
         StateInterface $inlineTranslation,
-        StoreManagerInterface $storeManager = null
+        StoreManagerInterface $storeManager = null,
+        GlobalForbiddenPatterns $forbiddenPatternsValidator
     ) {
         $this->contactsConfig = $contactsConfig;
         $this->transportBuilder = $transportBuilder;
         $this->inlineTranslation = $inlineTranslation;
         $this->storeManager = $storeManager ?: ObjectManager::getInstance()->get(StoreManagerInterface::class);
+        $this->forbiddenPatternsValidator = $forbiddenPatternsValidator;
     }
 
     /**
@@ -59,9 +70,24 @@ public function __construct(
      * @param string $replyTo
      * @param array $variables
      * @return void
+     * @throws LocalizedException
      */
     public function send($replyTo, array $variables)
     {
+        $validationErrors = [];
+        $fieldsToValidate = [
+            'name' => $variables['data']['name'] ?? '',
+            'comment' => $variables['data']['comment'] ?? '',
+            'email' => $variables['data']['email'] ?? '',
+        ];
+        $this->forbiddenPatternsValidator->validateData($fieldsToValidate, $validationErrors);
+
+        if (!empty($validationErrors)) {
+            throw new \Magento\Framework\Exception\LocalizedException(
+                __(implode("\n", $validationErrors))
+            );
+        }
+
         /** @see \Magento\Contact\Controller\Index\Post::validatedParams() */
         $replyToName = !empty($variables['data']['name']) ? $variables['data']['name'] : null;
 

app/code/Magento/Customer/Model/Validator/City.php

@@ -9,53 +9,42 @@
 
 use Magento\Customer\Model\Customer;
 use Magento\Framework\Validator\AbstractValidator;
+use Magento\Framework\Validator\GlobalCityValidator;
 
 /**
  * Customer city fields validator.
  */
 class City extends AbstractValidator
 {
     /**
-     * Allowed characters:
-     *
-     * \p{L}: Unicode letters.
-     * \p{M}: Unicode marks (diacritic marks, accents, etc.).
-     * ': Apostrophe mark.
-     * \s: Whitespace characters (spaces, tabs, newlines, etc.).
+     * @var GlobalCityValidator
      */
-    private const PATTERN_CITY = '/(?:[\p{L}\p{M}\s\-\']{1,100})/u';
+    private $cityValidator;
 
     /**
-     * Validate city fields.
+     * City constructor.
      *
-     * @param Customer $customer
-     * @return bool
+     * @param GlobalCityValidator $cityValidator
      */
-    public function isValid($customer)
+    public function __construct(GlobalCityValidator $cityValidator)
     {
-        if (!$this->isValidCity($customer->getCity())) {
-            parent::_addMessages([[
-                'city' => "Invalid City. Please use A-Z, a-z, 0-9, -, ', spaces"
-            ]]);
-        }
-
-        return count($this->_messages) == 0;
+        $this->cityValidator = $cityValidator;
     }
 
     /**
-     * Check if city field is valid.
+     * Validate city fields.
      *
-     * @param string|null $cityValue
+     * @param Customer $customer
      * @return bool
      */
-    private function isValidCity($cityValue)
+    public function isValid($customer): bool
     {
-        if ($cityValue != null) {
-            if (preg_match(self::PATTERN_CITY, $cityValue, $matches)) {
-                return $matches[0] == $cityValue;
-            }
+        if (!$this->cityValidator->isValidCity($customer->getCity())) {
+            parent::_addMessages([[
+                'city' => __("Invalid City. Please use only A-Z, a-z, 0-9, spaces, commas, -, ., ', &, [], ().")
+            ]]);
         }
 
-        return true;
+        return count($this->_messages) == 0;
     }
 }

app/code/Magento/Customer/Model/Validator/Name.php

@@ -9,13 +9,27 @@
 
 use Magento\Customer\Model\Customer;
 use Magento\Framework\Validator\AbstractValidator;
+use Magento\Framework\Validator\GlobalNameValidator;
 
 /**
  * Customer name fields validator.
  */
 class Name extends AbstractValidator
 {
-    private const PATTERN_NAME = '/(?:[\p{L}\p{M}\,\-\_\.\'’`&\s\d]){1,255}+/u';
+    /**
+     * @var GlobalNameValidator
+     */
+    private $nameValidator;
+
+    /**
+     * Name constructor.
+     *
+     * @param GlobalNameValidator $nameValidator
+     */
+    public function __construct(GlobalNameValidator $nameValidator)
+    {
+        $this->nameValidator = $nameValidator;
+    }
 
     /**
      * Validate name fields.
@@ -25,35 +39,18 @@ class Name extends AbstractValidator
      */
     public function isValid($customer)
     {
-        if (!$this->isValidName($customer->getFirstname())) {
-            parent::_addMessages([['firstname' => 'First Name is not valid!']]);
+        if (!$this->nameValidator->isValidName($customer->getFirstname())) {
+            parent::_addMessages([['firstname' => __('First Name is not valid!')]]);
         }
 
-        if (!$this->isValidName($customer->getLastname())) {
-            parent::_addMessages([['lastname' => 'Last Name is not valid!']]);
+        if (!$this->nameValidator->isValidName($customer->getLastname())) {
+            parent::_addMessages([['lastname' => __('Last Name is not valid!')]]);
         }
 
-        if (!$this->isValidName($customer->getMiddlename())) {
-            parent::_addMessages([['middlename' => 'Middle Name is not valid!']]);
+        if (!$this->nameValidator->isValidName($customer->getMiddlename())) {
+            parent::_addMessages([['middlename' => __('Middle Name is not valid!')]]);
         }
 
         return count($this->_messages) == 0;
     }
-
-    /**
-     * Check if name field is valid.
-     *
-     * @param string|null $nameValue
-     * @return bool
-     */
-    private function isValidName($nameValue)
-    {
-        if ($nameValue != null) {
-            if (preg_match(self::PATTERN_NAME, $nameValue, $matches)) {
-                return $matches[0] == $nameValue;
-            }
-        }
-
-        return true;
-    }
 }

app/code/Magento/Customer/Model/Validator/Street.php

@@ -9,60 +9,47 @@
 
 use Magento\Customer\Model\Customer;
 use Magento\Framework\Validator\AbstractValidator;
+use Magento\Framework\Validator\GlobalStreetValidator;
 
 /**
  * Customer street fields validator.
  */
 class Street extends AbstractValidator
 {
     /**
-     * Allowed characters:
+     * @var GlobalStreetValidator
+     */
+    private $streetValidator;
+
+    /**
+     * Street constructor.
      *
-     * \p{L}: Unicode letters.
-     * \p{M}: Unicode marks (diacritic marks, accents, etc.).
-     * ,: Comma.
-     * -: Hyphen.
-     * .: Period.
-     * `'’: Single quotes, both regular and right single quotation marks.
-     * &: Ampersand.
-     * \s: Whitespace characters (spaces, tabs, newlines, etc.).
-     * \d: Digits (0-9).
+     * @param GlobalStreetValidator $streetValidator
      */
-    private const PATTERN_STREET = "/(?:[\p{L}\p{M}\"[],-.'’`&\s\d]){1,255}+/u";
+    public function __construct(GlobalStreetValidator $streetValidator)
+    {
+        $this->streetValidator = $streetValidator;
+    }
 
     /**
      * Validate street fields.
      *
      * @param Customer $customer
      * @return bool
      */
-    public function isValid($customer)
+    public function isValid($customer): bool
     {
         foreach ($customer->getStreet() as $street) {
-            if (!$this->isValidStreet($street)) {
+            if (!$this->streetValidator->isValidStreet($street)) {
                 parent::_addMessages([[
-                    'street' => "Invalid Street Address. Please use A-Z, a-z, 0-9, , - . ' ’ ` & spaces"
+                    'street' => __(
+                        "Invalid Street Address. Please use only A-Z, a-z, 0-9, spaces, commas, -, ., ', " .
+                        "&, [], ()"
+                    )
                 ]]);
             }
         }
 
         return count($this->_messages) == 0;
     }
-
-    /**
-     * Check if street field is valid.
-     *
-     * @param string|null $streetValue
-     * @return bool
-     */
-    private function isValidStreet($streetValue)
-    {
-        if ($streetValue != null) {
-            if (preg_match(self::PATTERN_STREET, $streetValue, $matches)) {
-                return $matches[0] == $streetValue;
-            }
-        }
-
-        return true;
-    }
 }

app/code/Magento/Customer/Model/Validator/Telephone.php

@@ -9,22 +9,28 @@
 
 use Magento\Customer\Model\Customer;
 use Magento\Framework\Validator\AbstractValidator;
+use Magento\Framework\Validator\GlobalPhoneValidation;
 
 /**
  * Customer telephone fields validator.
  */
 class Telephone extends AbstractValidator
 {
     /**
-     * Allowed char:
+     * @var GlobalPhoneValidation
+     */
+    private $phoneValidator;
+
+    /**
+     * Telephone constructor.
      *
-     * \() :Matches open and close parentheses
-     * \+: Matches the plus sign.
-     * \-: Matches the hyphen.
-     * \d: Digits (0-9).
+     * @param GlobalPhoneValidation $phoneValidator
      */
-    private const PATTERN_TELEPHONE = '/(?:[\d\s\+\-\()]{1,20})/u';
-
+    public function __construct(GlobalPhoneValidation $phoneValidator)
+    {
+        $this->phoneValidator = $phoneValidator;
+    }
+
     /**
      * Validate telephone fields.
      *
@@ -33,29 +39,12 @@ class Telephone extends AbstractValidator
      */
     public function isValid($customer)
     {
-        if (!$this->isValidTelephone($customer->getTelephone())) {
+        if (!$this->phoneValidator->isValidPhone($customer->getTelephone())) {
             parent::_addMessages([[
-                'telephone' => "Invalid Phone Number. Please use 0-9, +, -, (, ) and space."
+                'telephone' => __('Invalid Phone Number. Please use 0-9, +, -, (), /, and space.')
             ]]);
         }
 
         return count($this->_messages) == 0;
     }
-
-    /**
-     * Check if telephone field is valid.
-     *
-     * @param string|null $telephoneValue
-     * @return bool
-     */
-    private function isValidTelephone($telephoneValue)
-    {
-        if ($telephoneValue != null) {
-            if (preg_match(self::PATTERN_TELEPHONE, (string) $telephoneValue, $matches)) {
-                return $matches[0] == $telephoneValue;
-            }
-        }
-
-        return true;
-    }
 }