Skip to content

Commit e4d2290

Browse files
kiatngsreicheladdison74Copilot
authored
New feature: enhance security with custom admin URL. (#4264)
* New feature: enhance security with custom admin URL. * Check if URL host matches custom admin URL. * Fixed incorrect 404 error skin when using dev/openmage/nginx-frontend.conf * Combined 2 if statements * Added validation of custom admin url * Removed unnecessary logic in store * Update app/code/core/Mage/Core/Controller/Varien/Router/Admin.php AI is correct! It is bad that str_contains() exposes a security vulnerability, "https://example.com/admin" would be able to access the admin panel because the host "example.com" may be contains in the admin URL "https://admin.example.com/". Co-authored-by: Copilot <[email protected]> * remove trim * refactor: improve custom admin URL handling and add redirect flag * Fix PHP-CS-Fixer * refactor: enhance admin domain validation in routing * Update app/code/core/Mage/Core/Controller/Varien/Router/Admin.php --------- Co-authored-by: Sven Reichel <[email protected]> Co-authored-by: Addison <[email protected]> Co-authored-by: Copilot <[email protected]>
1 parent 551a0f7 commit e4d2290

File tree

6 files changed

+119
-23
lines changed

6 files changed

+119
-23
lines changed

‎app/code/core/Mage/Adminhtml/Helper/Data.php‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ class Mage_Adminhtml_Helper_Data extends Mage_Adminhtml_Helper_Help_Mapping
1616
{
1717
public const XML_PATH_ADMINHTML_ROUTER_FRONTNAME = 'admin/routers/adminhtml/args/frontName';
1818
public const XML_PATH_USE_CUSTOM_ADMIN_URL = 'default/admin/url/use_custom';
19+
public const XML_PATH_CUSTOM_ADMIN_URL = 'default/admin/url/custom';
1920
public const XML_PATH_USE_CUSTOM_ADMIN_PATH = 'default/admin/url/use_custom_path';
2021
public const XML_PATH_CUSTOM_ADMIN_PATH = 'default/admin/url/custom_path';
2122
public const XML_PATH_ADMINHTML_SECURITY_USE_FORM_KEY = 'admin/security/use_form_key';
@@ -78,6 +79,20 @@ public static function getUrl($route = '', $params = [])
7879
return Mage::getModel('adminhtml/url')->getUrl($route, $params);
7980
}
8081

82+
/**
83+
* Get custom admin URL (validated and normalized when saved via admin panel)
84+
*/
85+
public static function getCustomAdminUrl(): string
86+
{
87+
$config = Mage::getConfig();
88+
89+
// Check if use custom admin URL is enabled
90+
$useCustom = (int) $config->getNode(self::XML_PATH_USE_CUSTOM_ADMIN_URL);
91+
return $useCustom
92+
? (string) $config->getNode(self::XML_PATH_CUSTOM_ADMIN_URL)
93+
: '';
94+
}
95+
8196
/**
8297
* @return false|int
8398
*/

‎app/code/core/Mage/Adminhtml/Model/System/Config/Backend/Admin/Custom.php‎

Lines changed: 71 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -23,19 +23,71 @@ class Mage_Adminhtml_Model_System_Config_Backend_Admin_Custom extends Mage_Core_
2323
public const XML_PATH_SECURE_BASE_LINK_URL = 'web/secure/base_link_url';
2424

2525
/**
26-
* Validate value before save
26+
* Validate custom admin URL before save
2727
*
2828
* @return $this
29+
* @throws Mage_Core_Exception
2930
*/
3031
protected function _beforeSave()
3132
{
32-
$value = $this->getValue();
33+
$value = trim($this->getValue());
34+
35+
// Empty is allowed (disabled custom admin URL)
36+
if (empty($value)) {
37+
$this->setValue($value);
38+
return $this;
39+
}
40+
41+
// Whitelist: only allow valid base URL characters
42+
// Valid for base URL: letters, numbers, and URL-safe characters (: / . - _ [ ])
43+
if (!preg_match('/^[a-zA-Z0-9:\/.\-_\[\]]+$/', $value)) {
44+
Mage::throwException(
45+
Mage::helper('adminhtml')->__('Custom Admin URL contains invalid characters.'),
46+
);
47+
}
48+
49+
// Parse the URL
50+
$urlParts = parse_url($value);
51+
52+
if ($urlParts === false) {
53+
Mage::throwException(
54+
Mage::helper('adminhtml')->__('Invalid Custom Admin URL format.'),
55+
);
56+
}
57+
58+
// Must have protocol
59+
if (!isset($urlParts['scheme'])) {
60+
Mage::throwException(
61+
Mage::helper('adminhtml')->__('Custom Admin URL must include protocol (http:// or https://).'),
62+
);
63+
}
64+
65+
// Only allow http and https
66+
if (!in_array($urlParts['scheme'], ['http', 'https'])) {
67+
Mage::throwException(
68+
Mage::helper('adminhtml')->__('Custom Admin URL must use http:// or https:// protocol.'),
69+
);
70+
}
71+
72+
// Must have hostname
73+
if (!isset($urlParts['host']) || empty($urlParts['host'])) {
74+
Mage::throwException(
75+
Mage::helper('adminhtml')->__('Custom Admin URL must include a hostname.'),
76+
);
77+
}
3378

34-
if (!empty($value) && !str_ends_with($value, '}}')) {
35-
$value = rtrim($value, '/') . '/';
79+
// Ensure trailing slash
80+
if (!str_ends_with($value, '/')) {
81+
$value .= '/';
3682
}
3783

3884
$this->setValue($value);
85+
86+
// Set redirect flag if custom admin URL changed
87+
if ($this->getOldValue() != $value) {
88+
Mage::register('custom_admin_path_redirect', true, true);
89+
}
90+
3991
return $this;
4092
}
4193

@@ -49,24 +101,24 @@ public function _afterSave()
49101
$useCustomUrl = $this->getData('groups/url/fields/use_custom/value');
50102
$value = $this->getValue();
51103

52-
if ($useCustomUrl == 1 && empty($value)) {
104+
// If use_custom is disabled OR value is empty, just save the value (don't update base URLs)
105+
if ($useCustomUrl != 1 || empty($value)) {
53106
return $this;
54107
}
55108

56-
if ($useCustomUrl == 1) {
57-
Mage::getConfig()->saveConfig(
58-
self::XML_PATH_SECURE_BASE_URL,
59-
$value,
60-
self::CONFIG_SCOPE,
61-
self::CONFIG_SCOPE_ID,
62-
);
63-
Mage::getConfig()->saveConfig(
64-
self::XML_PATH_UNSECURE_BASE_URL,
65-
$value,
66-
self::CONFIG_SCOPE,
67-
self::CONFIG_SCOPE_ID,
68-
);
69-
}
109+
// If use_custom is enabled AND value is not empty, update base URLs
110+
Mage::getConfig()->saveConfig(
111+
self::XML_PATH_SECURE_BASE_URL,
112+
$value,
113+
self::CONFIG_SCOPE,
114+
self::CONFIG_SCOPE_ID,
115+
);
116+
Mage::getConfig()->saveConfig(
117+
self::XML_PATH_UNSECURE_BASE_URL,
118+
$value,
119+
self::CONFIG_SCOPE,
120+
self::CONFIG_SCOPE_ID,
121+
);
70122

71123
return $this;
72124
}

‎app/code/core/Mage/Adminhtml/Model/System/Config/Backend/Admin/Usecustom.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,11 @@ protected function _afterSave()
5454
);
5555
}
5656

57+
// Set redirect flag if use custom admin URL changed
58+
if ($this->getOldValue() != $value) {
59+
Mage::register('custom_admin_path_redirect', true, true);
60+
}
61+
5762
return $this;
5863
}
5964
}

‎app/code/core/Mage/Core/Controller/Varien/Router/Admin.php‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,33 @@ protected function _getDefaultPath()
3636
}
3737

3838
/**
39-
* dummy call to pass through checking
39+
* Validate admin domain before routing
4040
*
4141
* @return bool
4242
*/
4343
protected function _beforeModuleMatch()
4444
{
45+
// Check if custom admin domain is configured
46+
if ($adminUrl = Mage_Adminhtml_Helper_Data::getCustomAdminUrl()) {
47+
$adminHost = parse_url($adminUrl, PHP_URL_HOST);
48+
if (!$adminHost) {
49+
// Should never happen - URL is validated when saved
50+
// If it does, fail secure (possible database corruption/bypass)
51+
Mage::log(
52+
"Unable to parse custom admin URL host: {$adminUrl}. Access denied for security.",
53+
Zend_Log::ERR,
54+
'system.log',
55+
);
56+
return false;
57+
}
58+
59+
$currentHost = $this->getFront()->getRequest()->getHttpHost();
60+
// Strip port for comparison (getHttpHost may include port)
61+
$currentHost = preg_replace('/:\d+$/', '', $currentHost);
62+
63+
return strtolower($adminHost) === strtolower($currentHost);
64+
}
65+
4566
return true;
4667
}
4768

‎app/locale/en_US/Mage_Adminhtml.csv‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,10 @@
281281
"Current Configuration Scope:","Current Configuration Scope:"
282282
"Current Month","Current Month"
283283
"Custom","Custom"
284+
"Custom Admin URL contains invalid characters.","Custom Admin URL contains invalid characters."
285+
"Custom Admin URL must include a hostname.","Custom Admin URL must include a hostname."
286+
"Custom Admin URL must include protocol (http:// or https://).","Custom Admin URL must include protocol (http:// or https://)."
287+
"Custom Admin URL must use http:// or https:// protocol.","Custom Admin URL must use http:// or https:// protocol."
284288
"Custom Variable ""%s""","Custom Variable ""%s"""
285289
"Custom Variables","Custom Variables"
286290
"Customer","Customer"
@@ -484,6 +488,7 @@
484488
"Insert Variable...","Insert Variable..."
485489
"Interactive","Interactive"
486490
"Interface Locale: %s","Interface Locale: %s"
491+
"Invalid Custom Admin URL format.","Invalid Custom Admin URL format."
487492
"Invalid Form Key. Please refresh the page.","Invalid Form Key. Please refresh the page."
488493
"Invalid Import Service Specified","Invalid Import Service Specified"
489494
"Invalid POST data (please check post_max_size and upload_max_filesize settings in your php.ini file).","Invalid POST data (please check post_max_size and upload_max_filesize settings in your php.ini file)."

‎errors/processor.php‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -489,9 +489,7 @@ protected function _validate(): bool
489489
*/
490490
protected function _setSkin(string $value, ?stdClass $config = null)
491491
{
492-
if (preg_match('/^[a-z0-9_]+$/i', $value)
493-
&& is_dir($this->_indexDir . self::ERROR_DIR . '/' . $value)
494-
) {
492+
if (preg_match('/^[a-z0-9_]+$/i', $value) && is_dir($this->_errorDir . $value)) {
495493
if (!$config && $this->_config) {
496494
$config = $this->_config;
497495
}

0 commit comments

Comments
 (0)