CVE-2023-41879: Use of Insufficiently Random Values (CWE-330)
value: Magento LTS is the official OpenMage LTS codebase. Guest orders may be viewed without authentication using a "guest-view" cookie which contains the order's "protect_code". This code is 6 hexadecimal characters which is arguably not enough to prevent a brute-force attack. Exposing each order would require a separate brute force attack. This issue has been patched in versions 19.5.1 and 20.1.1.
CWE description: The product uses insufficiently random numbers or values in a security context that depends on unpredictable numbers.
CVSS v3 base score: 7.5
Published: 2023-09-11T22:15Z
Repository: https://github.com/OpenMage/magento-lts
Language: PHP
Files changed: app/code/core/Mage/Core/Helper/Data.php, app/code/core/Mage/Core/etc/config.xml, app/code/core/Mage/Core/etc/system.xml, app/code/core/Mage/Sales/Helper/Guest.php, app/code/core/Mage/Sales/Model/Order.php, app/locale/en_US/Mage_Core.csv
Commit date: 2023-08-29 11:35:03 -0400
Commit message: Merge pull request from GHSA-9358-cpvx-c2qp

Co-authored-by: Fabrizio Balliano <fabrizio.balliano@gmail.com>
Diff: diff --git a/app/code/core/Mage/Core/Helper/Data.php b/app/code/core/Mage/Core/Helper/Data.php
index 2d1043a8dd2..d9955c26c7f 100644
--- a/app/code/core/Mage/Core/Helper/Data.php
+++ b/app/code/core/Mage/Core/Helper/Data.php
@@ -998,6 +998,41 @@ XML;
                 }
             }
         }
         return $data;
     }
+
+    /**
+     * @param bool $setErrorMessage Adds a predefined error message to the 'core/session' object
+     * @return bool
+     */
+    public function isRateLimitExceeded($setErrorMessage = true, $recordRateLimitHit = true): bool
+    {
+        $active = Mage::getStoreConfigFlag('system/rate_limit/active');
+        if ($active && $remoteAddr = Mage::helper('core/http')->getRemoteAddr()) {
+            $cacheTag = 'rate_limit_' . $remoteAddr;
+            if (Mage::app()->testCache($cacheTag)) {
+                $errorMessage = "Too Soon: You are trying to perform this operation too frequently. Please wait a few seconds and try again.";
+                Mage::getSingleton('core/session')->addError($this->__($errorMessage));
+                return true;
+            }
+
+            if ($recordRateLimitHit) {
+                $this->recordRateLimitHit();
+            }
+        }
+
+        return false;
+    }
+
+    /**
+     * @return void
+     */
+    public function recordRateLimitHit(): void
+    {
+        $active = Mage::getStoreConfigFlag('system/rate_limit/active');
+        if ($active && $remoteAddr = Mage::helper('core/http')->getRemoteAddr()) {
+            $cacheTag = 'rate_limit_' . $remoteAddr;
+            Mage::app()->saveCache(1, $cacheTag, ['brute_force'], Mage::getStoreConfig('system/rate_limit/timeframe'));
+        }
+    }
 }
diff --git a/app/code/core/Mage/Core/etc/config.xml b/app/code/core/Mage/Core/etc/config.xml
index ef593b10ba2..d7ce041219d 100644
--- a/app/code/core/Mage/Core/etc/config.xml
+++ b/app/code/core/Mage/Core/etc/config.xml
@@ -313,10 +313,14 @@
         </dev>
         <system>
             <csrf>
                 <use_form_key>1</use_form_key>
             </csrf>
+            <rate_limit>
+                <active>1</active>
+                <timeframe>30</timeframe>
+            </rate_limit>
             <cache>
                 <flush_cron_expr>30 2 * * *</flush_cron_expr>
             </cache>
             <smtp>
                 <disable>0</disable>
diff --git a/app/code/core/Mage/Core/etc/system.xml b/app/code/core/Mage/Core/etc/system.xml
index 9864433dedb..489b1e6fcf2 100644
--- a/app/code/core/Mage/Core/etc/system.xml
+++ b/app/code/core/Mage/Core/etc/system.xml
@@ -48,10 +48,37 @@
                             <show_in_website>1</show_in_website>
                             <show_in_store>1</show_in_store>
                         </use_form_key>
                     </fields>
                 </csrf>
+                <rate_limit translate="label comment" module="core">
+                    <label>Rate limit</label>
+                    <sort_order>10</sort_order>
+                    <show_in_default>1</show_in_default>
+                    <show_in_website>1</show_in_website>
+                    <show_in_store>1</show_in_store>
+                    <comment>This functionality limits the number of requests a user (identified by IP address) can perform within a specific time frame, preventing excessive resources usage and maintaining system performance, stability and security.</comment>
+                    <fields>
+                        <active translate="label">
+                            <label>Enabled</label>
+                            <frontend_type>select</frontend_type>
+                            <source_model>adminhtml/system_config_source_yesno</source_model>
+                            <sort_order>10</sort_order>
+                            <show_in_default>1</show_in_default>
+                            <show_in_website>1</show_in_website>
+                            <show_in_store>1</show_in_store>
+                        </active>
+                        <timeframe translate="label comment">
+                            <label>Timeframe</label>
+                            <sort_order>20</sort_order>
+                            <show_in_default>1</show_in_default>
+                            <show_in_website>1</show_in_website>
+                            <show_in_store>1</show_in_store>
+                            <comment>Number of seconds between each allowed request.</comment>
+                        </timeframe>
+                    </fields>
+                </rate_limit>
                 <cache translate="label" module="core">
                     <label>Advanced Cache Settings</label>
                     <sort_order>1000</sort_order>
                     <show_in_default>1</show_in_default>
                     <show_in_website>0</show_in_website>
@@ -67,54 +94,18 @@
                         </flush_cron_expr>
                     </fields>
                 </cache>
             </groups>
         </system>
-        <!--<web_track translate="label" module="core">
-            <label>Web Tracking</label>
-            <sort_order>180</sort_order>
-            <show_in_default>1</show_in_default>
-            <show_in_website>1</show_in_website>
-            <show_in_store>1</show_in_store>
-        </web_track>-->
         <advanced translate="label" module="core">
             <label>Advanced</label>
             <tab>advanced</tab>
             <sort_order>910</sort_order>
             <show_in_default>1</show_in_default>
             <show_in_website>1</show_in_website>
             <show_in_store>1</show_in_store>
             <groups>
-                <!--datashare translate="label">
-                    <label>Datasharing</label>
-                    <sort_order>1</sort_order>
-                    <show_in_default>1</show_in_default>
-                    <show_in_website>1</show_in_website>
-                    <show_in_store>0</show_in_store>
-                    <fields>
-                        <default translate="label">
-                            <label>Default</label>
-                            <frontend_type>multiselect</frontend_type>
-                            <backend_model>adminhtml/system_config_backend_datashare</backend_model>
-                            <source_model>adminhtml/system_config_source_store</source_model>
-                            <sort_order>1</sort_order>
-                            <show_in_default>1</show_in_default>
-                            <show_in_website>1</show_in_website>
-                            <show_in_store>1</show_in_store>
-                        </default>
-                        <default translate="label">
-                            <label>Default</label>
-                            <frontend_type>multiselect</frontend_type>
-                            <backend_model>adminhtml/system_config_backend_datashare</backend_model>
-                            <source_model>adminhtml/system_config_source_store</source_model>
-                            <sort_order>1</sort_order>
-                            <show_in_default>1</show_in_default>
-                            <show_in_website>1</show_in_website>
-                            <show_in_store>1</show_in_store>
-                        </default>
-                    </fields>
-                </datashare-->
                 <modules_disable_output translate="label">
                     <label>Disable Modules Output</label>
                     <frontend_model>adminhtml/system_config_form_fieldset_modules_disableOutput</frontend_model>
                     <sort_order>2</sort_order>
                     <show_in_default>1</show_in_default>
diff --git a/app/code/core/Mage/Sales/Helper/Guest.php b/app/code/core/Mage/Sales/Helper/Guest.php
index cbb841b5a5b..9d6099d9304 100644
--- a/app/code/core/Mage/Sales/Helper/Guest.php
+++ b/app/code/core/Mage/Sales/Helper/Guest.php
@@ -103,20 +103,24 @@ class Mage_Sales_Helper_Guest extends Mage_Core_Helper_Data
                 } else {
                     $errorMessage = 'Please log in to view your order details.';
                     $errors = true;
                 }
             } else {
+                Mage::helper('core')->recordRateLimitHit();
                 $errors = true;
             }
         }
 
         if (!$errors && $order->getId()) {
             Mage::register('current_order', $order);
             return true;
         }
 
-        Mage::getSingleton('core/session')->addError($this->__($errorMessage));
+        if (!Mage::helper('core')->isRateLimitExceeded(true, false)) {
+            Mage::getSingleton('core/session')->addError($this->__($errorMessage));
+        }
+
         Mage::app()->getResponse()->setRedirect(Mage::getUrl('sales/guest/form'));
         return false;
     }
 
     /**
diff --git a/app/code/core/Mage/Sales/Model/Order.php b/app/code/core/Mage/Sales/Model/Order.php
index ef55f66da25..ec00ea9bd88 100644
--- a/app/code/core/Mage/Sales/Model/Order.php
+++ b/app/code/core/Mage/Sales/Model/Order.php
@@ -2370,11 +2370,11 @@ class Mage_Sales_Model_Order extends Mage_Sales_Model_Abstract
         if ($this->hasShippingAddressId() && $this->getShippingAddressId() === null) {
             $this->unsShippingAddressId();
         }
 
         if (!$this->getId()) {
-            $this->setData('protect_code', substr(md5(uniqid(mt_rand(), true) . ':' . microtime(true)), 5, 6));
+            $this->setData('protect_code', Mage::helper('core')->getRandomString(16));
         }
 
         if ($this->getStatus() !== $this->getOrigData('status')) {
             Mage::dispatchEvent('order_status_changed_before_save', ['order' => $this]);
         }
diff --git a/app/locale/en_US/Mage_Core.csv b/app/locale/en_US/Mage_Core.csv
index 4322d2f9f70..91b9d95fadb 100644
--- a/app/locale/en_US/Mage_Core.csv
+++ b/app/locale/en_US/Mage_Core.csv
@@ -382,10 +382,11 @@
 "This will be displayed just before body closing tag.","This will be displayed just before body closing tag."
 "This will be included before head closing tag in page HTML.","This will be included before head closing tag in page HTML."
 "Timezone","Timezone"
 "Title Prefix","Title Prefix"
 "Title Suffix","Title Suffix"
+"Too Soon: You are trying to perform this operation too frequently. Please wait a few seconds and try again.","Too Soon: You are trying to perform this operation too frequently. Please wait a few seconds and try again."
 "Transactional Emails","Transactional Emails"
 "Translate Inline","Translate Inline"
 "Translate, blocks and other output caches should be disabled for both frontend and admin inline translations.","Translate, blocks and other output caches should be disabled for both frontend and admin inline translations."
 "Translations","Translations"
 "Two and more slashes together are not permitted in request path","Two and more slashes together are not permitted in request path"

Vulnerable code:         <!--<web_track translate="label" module="core">
            <label>Web Tracking</label>
            <sort_order>180</sort_order>
            <show_in_default>1</show_in_default>
            <show_in_website>1</show_in_website>
            <show_in_store>1</show_in_store>
        </web_track>-->
                <!--datashare translate="label">
                    <label>Datasharing</label>
                    <sort_order>1</sort_order>
                    <show_in_default>1</show_in_default>
                    <show_in_website>1</show_in_website>
                    <show_in_store>0</show_in_store>
                    <fields>
                        <default translate="label">
                            <label>Default</label>
                            <frontend_type>multiselect</frontend_type>
                            <backend_model>adminhtml/system_config_backend_datashare</backend_model>
                            <source_model>adminhtml/system_config_source_store</source_model>
                            <sort_order>1</sort_order>
                            <show_in_default>1</show_in_default>
                            <show_in_website>1</show_in_website>
                            <show_in_store>1</show_in_store>
                        </default>
                        <default translate="label">
                            <label>Default</label>
                            <frontend_type>multiselect</frontend_type>
                            <backend_model>adminhtml/system_config_backend_datashare</backend_model>
                            <source_model>adminhtml/system_config_source_store</source_model>
                            <sort_order>1</sort_order>
                            <show_in_default>1</show_in_default>
                            <show_in_website>1</show_in_website>
                            <show_in_store>1</show_in_store>
                        </default>
                    </fields>
                </datashare-->
        Mage::getSingleton('core/session')->addError($this->__($errorMessage));
            $this->setData('protect_code', substr(md5(uniqid(mt_rand(), true) . ':' . microtime(true)), 5, 6));
Fixed code: 
    /**
     * @param bool $setErrorMessage Adds a predefined error message to the 'core/session' object
     * @return bool
     */
    public function isRateLimitExceeded($setErrorMessage = true, $recordRateLimitHit = true): bool
    {
        $active = Mage::getStoreConfigFlag('system/rate_limit/active');
        if ($active && $remoteAddr = Mage::helper('core/http')->getRemoteAddr()) {
            $cacheTag = 'rate_limit_' . $remoteAddr;
            if (Mage::app()->testCache($cacheTag)) {
                $errorMessage = "Too Soon: You are trying to perform this operation too frequently. Please wait a few seconds and try again.";
                Mage::getSingleton('core/session')->addError($this->__($errorMessage));
                return true;
            }

            if ($recordRateLimitHit) {
                $this->recordRateLimitHit();
            }
        }

        return false;
    }

    /**
     * @return void
     */
    public function recordRateLimitHit(): void
    {
        $active = Mage::getStoreConfigFlag('system/rate_limit/active');
        if ($active && $remoteAddr = Mage::helper('core/http')->getRemoteAddr()) {
            $cacheTag = 'rate_limit_' . $remoteAddr;
            Mage::app()->saveCache(1, $cacheTag, ['brute_force'], Mage::getStoreConfig('system/rate_limit/timeframe'));
        }
    }
            <rate_limit>
                <active>1</active>
                <timeframe>30</timeframe>
            </rate_limit>
                <rate_limit translate="label comment" module="core">
                    <label>Rate limit</label>
                    <sort_order>10</sort_order>
                    <show_in_default>1</show_in_default>
                    <show_in_website>1</show_in_website>
                    <show_in_store>1</show_in_store>
                    <comment>This functionality limits the number of requests a user (identified by IP address) can perform within a specific time frame, preventing excessive resources usage and maintaining system performance, stability and security.</comment>
                    <fields>
                        <active translate="label">
                            <label>Enabled</label>
                            <frontend_type>select</frontend_type>
                            <source_model>adminhtml/system_config_source_yesno</source_model>
                            <sort_order>10</sort_order>
                            <show_in_default>1</show_in_default>
                            <show_in_website>1</show_in_website>
                            <show_in_store>1</show_in_store>
                        </active>
                        <timeframe translate="label comment">
                            <label>Timeframe</label>
                            <sort_order>20</sort_order>
                            <show_in_default>1</show_in_default>
                            <show_in_website>1</show_in_website>
                            <show_in_store>1</show_in_store>
                            <comment>Number of seconds between each allowed request.</comment>
                        </timeframe>
                    </fields>
                </rate_limit>
                Mage::helper('core')->recordRateLimitHit();
        if (!Mage::helper('core')->isRateLimitExceeded(true, false)) {
            Mage::getSingleton('core/session')->addError($this->__($errorMessage));
        }

            $this->setData('protect_code', Mage::helper('core')->getRandomString(16));
"Too Soon: You are trying to perform this operation too frequently. Please wait a few seconds and try again.","Too Soon: You are trying to perform this operation too frequently. Please wait a few seconds and try again."