From 71e1479c2b517cee9307196cc54a5e96c66bb148 Mon Sep 17 00:00:00 2001 From: Sven Reichel Date: Wed, 27 Nov 2024 09:58:34 +0100 Subject: [PATCH] Obfuscated login credentials in stacktraces (#4388) * Obfuscated login credentials * rector --- .phpstan.dist.baseline.neon | 10 ---- app/code/core/Mage/Admin/Model/Session.php | 3 ++ app/code/core/Mage/Admin/Model/User.php | 6 +++ .../Api/Model/Server/Handler/Abstract.php | 53 ++++++++++++------- .../Mage/Api/Model/Server/Wsi/Handler.php | 16 +++--- app/code/core/Mage/Api/Model/Session.php | 3 ++ app/code/core/Mage/Api/Model/User.php | 3 ++ .../Mage/Core/Model/Security/Obfuscated.php | 37 +++++++++++++ app/code/core/Mage/Customer/Model/Session.php | 3 ++ lib/Mage/System/Ftp.php | 3 ++ 10 files changed, 103 insertions(+), 34 deletions(-) create mode 100644 app/code/core/Mage/Core/Model/Security/Obfuscated.php diff --git a/.phpstan.dist.baseline.neon b/.phpstan.dist.baseline.neon index 360b0969a54..00de356bd16 100644 --- a/.phpstan.dist.baseline.neon +++ b/.phpstan.dist.baseline.neon @@ -990,11 +990,6 @@ parameters: count: 1 path: app/code/core/Mage/Api/Model/Server/Wsi/Adapter/Soap.php - - - message: "#^Parameter \\#1 \\$request \\(stdClass\\) of method Mage_Api_Model_Server_Wsi_Handler\\:\\:endSession\\(\\) should be compatible with parameter \\$sessionId \\(string\\) of method Mage_Api_Model_Server_Handler_Abstract\\:\\:endSession\\(\\)$#" - count: 1 - path: app/code/core/Mage/Api/Model/Server/Wsi/Handler.php - - message: "#^Parameter \\#1 \\$string of function strlen expects string, array given\\.$#" count: 1 @@ -1010,11 +1005,6 @@ parameters: count: 1 path: app/code/core/Mage/Api/Model/Server/Wsi/Handler.php - - - message: "#^Return type \\(stdClass\\) of method Mage_Api_Model_Server_Wsi_Handler\\:\\:login\\(\\) should be compatible with return type \\(string\\) of method Mage_Api_Model_Server_Handler_Abstract\\:\\:login\\(\\)$#" - count: 1 - path: app/code/core/Mage/Api/Model/Server/Wsi/Handler.php - - message: "#^Return type \\(bool\\) of method Mage_Api_Model_Session\\:\\:clear\\(\\) should be compatible with return type \\(\\$this\\(Mage_Core_Model_Session_Abstract_Varien\\)\\) of method Mage_Core_Model_Session_Abstract_Varien\\:\\:clear\\(\\)$#" count: 1 diff --git a/app/code/core/Mage/Admin/Model/Session.php b/app/code/core/Mage/Admin/Model/Session.php index 8ac0c92c878..03e23406d1b 100644 --- a/app/code/core/Mage/Admin/Model/Session.php +++ b/app/code/core/Mage/Admin/Model/Session.php @@ -148,6 +148,9 @@ public function login($username, $password, $request = null) return null; } + $username = new Mage_Core_Model_Security_Obfuscated($username); + $password = new Mage_Core_Model_Security_Obfuscated($password); + try { /** @var Mage_Admin_Model_User $user */ $user = $this->_factory->getModel('admin/user'); diff --git a/app/code/core/Mage/Admin/Model/User.php b/app/code/core/Mage/Admin/Model/User.php index f78537318d1..ee069c5879c 100644 --- a/app/code/core/Mage/Admin/Model/User.php +++ b/app/code/core/Mage/Admin/Model/User.php @@ -374,6 +374,9 @@ public function getAclRole() */ public function authenticate($username, $password) { + $username = new Mage_Core_Model_Security_Obfuscated($username); + $password = new Mage_Core_Model_Security_Obfuscated($password); + $config = Mage::getStoreConfigFlag('admin/security/use_case_sensitive_login'); $result = false; @@ -427,6 +430,9 @@ public function validatePasswordHash(string $string1, string $string2): bool */ public function login($username, $password) { + $username = new Mage_Core_Model_Security_Obfuscated($username); + $password = new Mage_Core_Model_Security_Obfuscated($password); + if ($this->authenticate($username, $password)) { $this->getResource()->recordLogin($this); Mage::getSingleton('core/session')->renewFormKey(); diff --git a/app/code/core/Mage/Api/Model/Server/Handler/Abstract.php b/app/code/core/Mage/Api/Model/Server/Handler/Abstract.php index 50fbc2b441b..1cfd281dc9c 100644 --- a/app/code/core/Mage/Api/Model/Server/Handler/Abstract.php +++ b/app/code/core/Mage/Api/Model/Server/Handler/Abstract.php @@ -91,7 +91,7 @@ protected function _startSession($sessionId = null) /** * Allow insta-login via HTTP Basic Auth * - * @param string $sessionId + * @param stdClass|string|null $sessionId * @return $this * @SuppressWarnings(PHPMD.Superglobals) */ @@ -204,20 +204,25 @@ protected function _prepareResourceModelName($resource) * Login user and Retrieve session id * * @param string $username - * @param string $apiKey - * @return string + * @param string|null $apiKey + * @return stdClass|string|void */ public function login($username, $apiKey = null) { if (empty($username) || empty($apiKey)) { - return $this->_fault('invalid_request_param'); + $this->_fault('invalid_request_param'); + return; } + $username = new Mage_Core_Model_Security_Obfuscated($username); + $apiKey = new Mage_Core_Model_Security_Obfuscated($apiKey); + try { $this->_startSession(); $this->_getSession()->login($username, $apiKey); } catch (Exception $e) { - return $this->_fault('access_denied'); + $this->_fault('access_denied'); + return; } return $this->_getSession()->getSessionId(); } @@ -228,7 +233,7 @@ public function login($username, $apiKey = null) * @param string $sessionId * @param string $apiPath * @param array $args - * @return mixed + * @return mixed|void */ public function call($sessionId, $apiPath, $args = []) { @@ -236,13 +241,15 @@ public function call($sessionId, $apiPath, $args = []) ->_startSession($sessionId); if (!$this->_getSession()->isLoggedIn($sessionId)) { - return $this->_fault('session_expired'); + $this->_fault('session_expired'); + return; } list($resourceName, $methodName) = explode('.', $apiPath); if (empty($resourceName) || empty($methodName)) { - return $this->_fault('resource_path_invalid'); + $this->_fault('resource_path_invalid'); + return; } $resourcesAlias = $this->_getConfig()->getResourcesAlias(); @@ -254,21 +261,24 @@ public function call($sessionId, $apiPath, $args = []) if (!isset($resources->$resourceName) || !isset($resources->$resourceName->methods->$methodName) ) { - return $this->_fault('resource_path_invalid'); + $this->_fault('resource_path_invalid'); + return; } if (!isset($resources->$resourceName->public) && isset($resources->$resourceName->acl) && !$this->_isAllowed((string)$resources->$resourceName->acl) ) { - return $this->_fault('access_denied'); + $this->_fault('access_denied'); + return; } if (!isset($resources->$resourceName->methods->$methodName->public) && isset($resources->$resourceName->methods->$methodName->acl) && !$this->_isAllowed((string)$resources->$resourceName->methods->$methodName->acl) ) { - return $this->_fault('access_denied'); + $this->_fault('access_denied'); + return; } $methodInfo = $resources->$resourceName->methods->$methodName; @@ -302,10 +312,12 @@ public function call($sessionId, $apiPath, $args = []) throw new Mage_Api_Exception('resource_path_not_callable'); } } catch (Mage_Api_Exception $e) { - return $this->_fault($e->getMessage(), $resourceName, $e->getCustomMessage()); + $this->_fault($e->getMessage(), $resourceName, $e->getCustomMessage()); + return; } catch (Exception $e) { Mage::logException($e); - return $this->_fault('internal', null, $e->getMessage()); + $this->_fault('internal', null, $e->getMessage()); + return; } } @@ -322,7 +334,8 @@ public function multiCall($sessionId, array $calls = [], $options = []) ->_startSession($sessionId); if (!$this->_getSession()->isLoggedIn($sessionId)) { - return $this->_fault('session_expired'); + $this->_fault('session_expired'); + return; } $result = []; @@ -454,7 +467,8 @@ public function resources($sessionId) ->_startSession($sessionId); if (!$this->_getSession()->isLoggedIn($sessionId)) { - return $this->_fault('session_expired'); + $this->_fault('session_expired'); + return; } $resources = []; @@ -519,7 +533,8 @@ public function resourceFaults($sessionId, $resourceName) ->_startSession($sessionId); if (!$this->_getSession()->isLoggedIn($sessionId)) { - return $this->_fault('session_expired'); + $this->_fault('session_expired'); + return; } $resourcesAlias = $this->_getConfig()->getResourcesAlias(); @@ -532,13 +547,15 @@ public function resourceFaults($sessionId, $resourceName) if (empty($resourceName) || !isset($resources->$resourceName) ) { - return $this->_fault('resource_path_invalid'); + $this->_fault('resource_path_invalid'); + return; } if (isset($resources->$resourceName->acl) && !$this->_isAllowed((string)$resources->$resourceName->acl) ) { - return $this->_fault('access_denied'); + $this->_fault('access_denied'); + return; } return array_values($this->_getConfig()->getFaults($resourceName)); diff --git a/app/code/core/Mage/Api/Model/Server/Wsi/Handler.php b/app/code/core/Mage/Api/Model/Server/Wsi/Handler.php index a1144a4d434..33643d8f211 100644 --- a/app/code/core/Mage/Api/Model/Server/Wsi/Handler.php +++ b/app/code/core/Mage/Api/Model/Server/Wsi/Handler.php @@ -77,7 +77,7 @@ public function __call($function, $args) * Login user and Retrieve session id * * @param string $username - * @param string $apiKey + * @param string|null $apiKey * @return stdClass */ public function login($username, $apiKey = null) @@ -87,6 +87,9 @@ public function login($username, $apiKey = null) $username = $username->username; } + $username = new Mage_Core_Model_Security_Obfuscated($username); + $apiKey = is_null($apiKey) ? null : new Mage_Core_Model_Security_Obfuscated($apiKey); + $stdObject = new stdClass(); $stdObject->result = parent::login($username, $apiKey); return $stdObject; @@ -96,14 +99,15 @@ public function login($username, $apiKey = null) * Return called class and method names * * @param String $apiPath - * @return array + * @return array|void */ protected function _getResourceName($apiPath) { list($resourceName, $methodName) = explode('.', $apiPath); if (empty($resourceName) || empty($methodName)) { - return $this->_fault('resource_path_invalid'); + $this->_fault('resource_path_invalid'); + return; } $resourcesAlias = $this->_getConfig()->getResourcesAlias(); @@ -165,13 +169,13 @@ public function prepareArgs($params, $args) /** * End web service session * - * @param stdClass $request + * @param stdClass|string $sessionId * @return stdClass */ - public function endSession($request) + public function endSession($sessionId) { $stdObject = new stdClass(); - $stdObject->result = parent::endSession($request->sessionId); + $stdObject->result = parent::endSession($sessionId->sessionId); return $stdObject; } } diff --git a/app/code/core/Mage/Api/Model/Session.php b/app/code/core/Mage/Api/Model/Session.php index af585683951..f9e272ee069 100644 --- a/app/code/core/Mage/Api/Model/Session.php +++ b/app/code/core/Mage/Api/Model/Session.php @@ -124,6 +124,9 @@ public function getIsInstaLogin(): bool */ public function login($username, $apiKey) { + $username = new Mage_Core_Model_Security_Obfuscated($username); + $apiKey = new Mage_Core_Model_Security_Obfuscated($apiKey); + $user = Mage::getModel('api/user') ->setSessid($this->getSessionId()); if ($this->getIsInstaLogin() && $user->authenticate($username, $apiKey)) { diff --git a/app/code/core/Mage/Api/Model/User.php b/app/code/core/Mage/Api/Model/User.php index bbd93da3173..1e50dab74fc 100644 --- a/app/code/core/Mage/Api/Model/User.php +++ b/app/code/core/Mage/Api/Model/User.php @@ -263,6 +263,9 @@ public function authenticate($username, $apiKey) */ public function login($username, $apiKey) { + $username = new Mage_Core_Model_Security_Obfuscated($username); + $apiKey = new Mage_Core_Model_Security_Obfuscated($apiKey); + $sessId = $this->getSessid(); if ($this->authenticate($username, $apiKey)) { $this->setSessid($sessId); diff --git a/app/code/core/Mage/Core/Model/Security/Obfuscated.php b/app/code/core/Mage/Core/Model/Security/Obfuscated.php new file mode 100644 index 00000000000..c6cfd633d12 --- /dev/null +++ b/app/code/core/Mage/Core/Model/Security/Obfuscated.php @@ -0,0 +1,37 @@ +value = $value; + } + + public function __toString(): string + { + return $this->value; + } +} diff --git a/app/code/core/Mage/Customer/Model/Session.php b/app/code/core/Mage/Customer/Model/Session.php index 10708d940c7..865093de6f3 100644 --- a/app/code/core/Mage/Customer/Model/Session.php +++ b/app/code/core/Mage/Customer/Model/Session.php @@ -227,6 +227,9 @@ public function checkCustomerId($customerId) */ public function login($username, $password) { + $username = new Mage_Core_Model_Security_Obfuscated($username); + $password = new Mage_Core_Model_Security_Obfuscated($password); + /** @var Mage_Customer_Model_Customer $customer */ $customer = Mage::getModel('customer/customer') ->setWebsiteId(Mage::app()->getStore()->getWebsiteId()); diff --git a/lib/Mage/System/Ftp.php b/lib/Mage/System/Ftp.php index cf30a0e760d..fec8ffd85f2 100644 --- a/lib/Mage/System/Ftp.php +++ b/lib/Mage/System/Ftp.php @@ -98,6 +98,9 @@ public function mkdirRecursive($path, $mode = 0777) */ public function login($login = 'anonymous', $password = 'test@gmail.com') { + $login = new Mage_Core_Model_Security_Obfuscated($login); + $password = new Mage_Core_Model_Security_Obfuscated($password); + $this->checkConnected(); $res = @ftp_login($this->_conn, $login, $password); if (!$res) {