From c6850c6c3df1f857fb9274457ac356a9a3a84395 Mon Sep 17 00:00:00 2001 From: the-djmaze <> Date: Tue, 1 Nov 2022 17:02:49 +0100 Subject: [PATCH] Bugfix: logger leaked some passwords --- .../app/libraries/MailSo/Imap/ImapClient.php | 71 +++++-------------- .../0.0.0/app/libraries/MailSo/Log/Logger.php | 7 +- .../app/libraries/MailSo/Net/NetClient.php | 47 +++++------- .../MailSo/Sieve/ManageSieveClient.php | 15 ++-- .../app/libraries/MailSo/Smtp/SmtpClient.php | 15 ++-- 5 files changed, 53 insertions(+), 102 deletions(-) diff --git a/snappymail/v/0.0.0/app/libraries/MailSo/Imap/ImapClient.php b/snappymail/v/0.0.0/app/libraries/MailSo/Imap/ImapClient.php index 0a46ec57b..615cf2c79 100644 --- a/snappymail/v/0.0.0/app/libraries/MailSo/Imap/ImapClient.php +++ b/snappymail/v/0.0.0/app/libraries/MailSo/Imap/ImapClient.php @@ -170,10 +170,8 @@ class ImapClient extends \MailSo\Net\NetClient $sAuthzid = $this->getResponseValue($this->SendRequestGetResponse('AUTHENTICATE', array($type)), Enumerations\ResponseType::CONTINUATION); $this->sendRaw($SASL->authenticate($sLogin, $sPassword/*, $sAuthzid*/), true); $sChallenge = $SASL->challenge($this->getResponseValue($this->getResponse(), Enumerations\ResponseType::CONTINUATION)); - if ($this->oLogger) { - $this->oLogger->AddSecret($sChallenge); - } - $this->sendRaw($sChallenge, true, '*******'); + $this->oLogger && $this->oLogger->AddSecret($sChallenge); + $this->sendRaw($sChallenge); $oResponse = $this->getResponse(); $SASL->verify($this->getResponseValue($oResponse)); } @@ -182,31 +180,26 @@ class ImapClient extends \MailSo\Net\NetClient $sChallenge = $this->getResponseValue($this->SendRequestGetResponse('AUTHENTICATE', array($type)), Enumerations\ResponseType::CONTINUATION); $this->oLogger->Write('challenge: '.\base64_decode($sChallenge)); $sAuth = $SASL->authenticate($sLogin, $sPassword, $sChallenge); - if ($this->oLogger) { - $this->oLogger->AddSecret($sAuth); - } - $this->sendRaw($sAuth, true, '*******'); + $this->oLogger && $this->oLogger->AddSecret($sAuth); + $this->sendRaw($sAuth); $oResponse = $this->getResponse(); } else if ('PLAIN' === $type || 'OAUTHBEARER' === $type) { $sAuth = $SASL->authenticate($sLogin, $sPassword); - if ($this->oLogger) { - $this->oLogger->AddSecret($sAuth); - } + $this->oLogger && $this->oLogger->AddSecret($sAuth); if ($this->IsSupported('SASL-IR')) { $oResponse = $this->SendRequestGetResponse('AUTHENTICATE', array($type, $sAuth)); } else { $this->SendRequestGetResponse('AUTHENTICATE', array($type)); - $this->sendRaw($sAuth, true, '*******'); + $this->sendRaw($sAuth); $oResponse = $this->getResponse(); } } - else if ('XOAUTH2' === $type) + else if ('XOAUTH2' === $type || 'OAUTHBEARER' === $type) { $sAuth = $SASL->authenticate($sLogin, $sPassword); - $this->SendRequest('AUTHENTICATE', array($type, $sAuth)); - $oResponse = $this->getResponse(); + $oResponse = $this->SendRequestGetResponse('AUTHENTICATE', array($type, $sAuth)); $oR = $oResponse->getLast(); if ($oR && Enumerations\ResponseType::CONTINUATION === $oR->ResponseType) { if (!empty($oR->ResponseList[1]) && preg_match('/^[a-zA-Z0-9=+\/]+$/', $oR->ResponseList[1])) { @@ -223,19 +216,14 @@ class ImapClient extends \MailSo\Net\NetClient $this->sendRaw($SASL->authenticate($sLogin, $sPassword, $sB64), true); $this->getResponse(); $sPass = $SASL->challenge(''/*UGFzc3dvcmQ6*/); - if ($this->oLogger) { - $this->oLogger->AddSecret($sPass); - } - $this->sendRaw($sPass, true, '*******'); + $this->oLogger && $this->oLogger->AddSecret($sPass); + $this->sendRaw($sPass); $oResponse = $this->getResponse(); } else { $sPassword = $this->EscapeString(\mb_convert_encoding($sPassword, 'ISO-8859-1', 'UTF-8')); - if ($this->oLogger) - { - $this->oLogger->AddSecret($sPassword); - } + $this->oLogger && $this->oLogger->AddSecret($sPassword); $oResponse = $this->SendRequestGetResponse('LOGIN', array( $this->EscapeString($sLogin), @@ -425,11 +413,8 @@ class ImapClient extends \MailSo\Net\NetClient public function SendRequest(string $sCommand, array $aParams = array(), bool $bBreakOnLiteral = false) : string { $sCommand = \trim($sCommand); - if (!\strlen($sCommand)) - { - $this->writeLogException( - new \InvalidArgumentException, - \LOG_ERR, true); + if (!\strlen($sCommand)) { + $this->writeLogException(new \InvalidArgumentException, \LOG_ERR, true); } $this->IsConnected(true); @@ -438,42 +423,20 @@ class ImapClient extends \MailSo\Net\NetClient $sRealCommand = $sTag.' '.$sCommand.$this->prepareParamLine($aParams); - $sFakeCommand = ''; - if ($aFakeParams = $this->secureRequestParams($sCommand, $aParams)) { - $sFakeCommand = $sTag.' '.$sCommand.$this->prepareParamLine($aFakeParams); - } - -// $this->lastCommand = $sFakeCommand ?: $sRealCommand; - $this->aTagTimeouts[$sTag] = \microtime(true); - if ($bBreakOnLiteral && !\preg_match('/\d\+\}\r\n/', $sRealCommand)) - { + if ($bBreakOnLiteral && !\preg_match('/\d\+\}\r\n/', $sRealCommand)) { $iPos = \strpos($sRealCommand, "}\r\n"); - if (false !== $iPos) - { - $iFakePos = \strpos($sFakeCommand, "}\r\n"); - - $this->sendRaw(\substr($sRealCommand, 0, $iPos + 1), true, - false !== $iFakePos ? \substr($sFakeCommand, 0, $iFakePos + 3) : ''); - + if (false !== $iPos) { + $this->sendRaw(\substr($sRealCommand, 0, $iPos + 1)); return \substr($sRealCommand, $iPos + 3); } } - $this->sendRaw($sRealCommand, true, $sFakeCommand); + $this->sendRaw($sRealCommand); return ''; } - protected function secureRequestParams(string $sCommand, array $aParams) : ?array - { - if ('LOGIN' === $sCommand && isset($aParams[1])) { - $aParams[1] = '"********"'; - return $aParams; - } - return null; - } - /** * @throws \InvalidArgumentException * @throws \MailSo\RuntimeException diff --git a/snappymail/v/0.0.0/app/libraries/MailSo/Log/Logger.php b/snappymail/v/0.0.0/app/libraries/MailSo/Log/Logger.php index 74ad135df..ecec59391 100644 --- a/snappymail/v/0.0.0/app/libraries/MailSo/Log/Logger.php +++ b/snappymail/v/0.0.0/app/libraries/MailSo/Log/Logger.php @@ -66,8 +66,9 @@ class Logger extends \SplFixedArray public function AddSecret(string $sWord) : void { - $sWord = \strlen(\trim($sWord)); - if ($sWord) { +// $this->bShowSecrets && $this->Write("AddSecret '{$sWord}'", \LOG_INFO, '', false); + $sWord = \trim($sWord); + if (\strlen($sWord)) { $this->aSecretWords[] = $sWord; $this->aSecretWords = \array_unique($this->aSecretWords); } @@ -165,7 +166,7 @@ class Logger extends \SplFixedArray $this->bUsed = true; - if ($bSearchSecretWords && !$this->bShowSecrets && \count($this->aSecretWords)) + if ($bSearchSecretWords && !$this->bShowSecrets && $this->aSecretWords) { $sDesc = \str_replace($this->aSecretWords, '*******', $sDesc); } diff --git a/snappymail/v/0.0.0/app/libraries/MailSo/Net/NetClient.php b/snappymail/v/0.0.0/app/libraries/MailSo/Net/NetClient.php index 0c0beb928..ed0e78ba4 100644 --- a/snappymail/v/0.0.0/app/libraries/MailSo/Net/NetClient.php +++ b/snappymail/v/0.0.0/app/libraries/MailSo/Net/NetClient.php @@ -160,8 +160,7 @@ abstract class NetClient } $this->iStartConnectTime = \microtime(true); - $this->writeLog('Start connection to "'.$this->sConnectedHost.':'.$this->iConnectedPort.'"', - \LOG_INFO); + $this->writeLog('Start connection to "'.$this->sConnectedHost.':'.$this->iConnectedPort.'"'); $aStreamContextSettings = array( 'ssl' => $oSettings->ssl @@ -186,8 +185,7 @@ abstract class NetClient \restore_error_handler(); - $this->writeLog('Connect ('.($this->rConnect ? 'success' : 'failed').')', - \LOG_INFO); + $this->writeLog('Connect ('.($this->rConnect ? 'success' : 'failed').')'); if (!$this->rConnect) { @@ -253,7 +251,7 @@ abstract class NetClient $bResult = \fclose($this->rConnect); $this->writeLog('Disconnected from "'.$this->sConnectedHost.':'.$this->iConnectedPort.'" ('. - (($bResult) ? 'success' : 'unsuccess').')', \LOG_INFO); + (($bResult) ? 'success' : 'unsuccess').')'); if ($this->iStartConnectTime) { @@ -301,21 +299,14 @@ abstract class NetClient */ protected function sendRaw(string $sRaw, bool $bWriteToLog = true, string $sFakeRaw = '') : void { - if ($this->bUnreadBuffer) - { + if ($this->bUnreadBuffer) { $this->writeLogException(new Exceptions\SocketUnreadBufferException, \LOG_ERR, true); } - $bFake = \strlen($sFakeRaw); $sRaw .= "\r\n"; - if ($this->oLogger && $this->oLogger->ShowSecrets()) - { - $bFake = false; - } - - if ($bFake) - { + $bFake = \strlen($sFakeRaw) && $this->oLogger && !$this->oLogger->ShowSecrets(); + if ($bFake) { $sFakeRaw .= "\r\n"; } @@ -327,7 +318,7 @@ abstract class NetClient } else if ($bWriteToLog) { - $this->writeLogWithCrlf('> '.($bFake ? $sFakeRaw : $sRaw), \LOG_INFO); + $this->writeLogWithCrlf('> '.($bFake ? $sFakeRaw : $sRaw)); } } @@ -373,8 +364,7 @@ abstract class NetClient } else { - $this->writeLog('Stream Meta: '. - \print_r($aSocketStatus, true), \LOG_ERR); + $this->writeLog('Stream Meta: '.\print_r($aSocketStatus, true), \LOG_ERR); $this->writeLogException( new Exceptions\SocketReadException, @@ -389,36 +379,31 @@ abstract class NetClient $iLimit = 5000; // 5KB if ($iLimit < $iReadedLen) { - $this->writeLogWithCrlf('[cutted:'.$iReadedLen.'] < '.\substr($this->sResponseBuffer, 0, $iLimit).'...', - \LOG_INFO); + $this->writeLogWithCrlf('[cutted:'.$iReadedLen.'] < '.\substr($this->sResponseBuffer, 0, $iLimit).'...'); } else { - $this->writeLogWithCrlf('< '.$this->sResponseBuffer, //.' ['.$iReadedLen.']', - \LOG_INFO); +// $this->writeLogWithCrlf('< '.$this->sResponseBuffer, //.' ['.$iReadedLen.']', + $this->writeLogWithCrlf('< '.$this->sResponseBuffer); } } else { - $this->writeLog('Received '.$iReadedLen.'/'.$iReadLen.' bytes.', - \LOG_INFO); + $this->writeLog('Received '.$iReadedLen.'/'.$iReadLen.' bytes.'); } } } abstract function getLogName() : string; - protected function writeLog(string $sDesc, int $iDescType = \LOG_INFO, bool $bDiplayCrLf = false) : void + protected function writeLog(string $sDesc, int $iDescType = \LOG_INFO) : void { - if ($this->oLogger) - { - $this->oLogger->Write($sDesc, $iDescType, $this->getLogName(), true, $bDiplayCrLf); - } + $this->oLogger && $this->oLogger->Write($sDesc, $iDescType, $this->getLogName()); } - protected function writeLogWithCrlf(string $sDesc, int $iDescType = \LOG_INFO) : void + protected function writeLogWithCrlf(string $sDesc) : void { - $this->writeLog($sDesc, $iDescType, true); + $this->oLogger && $this->oLogger->Write($sDesc, \LOG_INFO, $this->getLogName(), true, true); } protected function writeLogException(\Throwable $oException, diff --git a/snappymail/v/0.0.0/app/libraries/MailSo/Sieve/ManageSieveClient.php b/snappymail/v/0.0.0/app/libraries/MailSo/Sieve/ManageSieveClient.php index 7f9bbf0fc..c67ca7c7a 100644 --- a/snappymail/v/0.0.0/app/libraries/MailSo/Sieve/ManageSieveClient.php +++ b/snappymail/v/0.0.0/app/libraries/MailSo/Sieve/ManageSieveClient.php @@ -143,10 +143,8 @@ class ManageSieveClient extends \MailSo\Net\NetClient $sAuthzid = $this->getResponseValue($this->SendRequestGetResponse('AUTHENTICATE', array($type)), \MailSo\Imap\Enumerations\ResponseType::CONTINUATION); $this->sendRaw($SASL->authenticate($sLogin, $sPassword/*, $sAuthzid* /), true); $sChallenge = $SASL->challenge($this->getResponseValue($this->getResponse(), \MailSo\Imap\Enumerations\ResponseType::CONTINUATION)); - if ($this->oLogger) { - $this->oLogger->AddSecret($sChallenge); - } - $this->sendRaw($sChallenge, true, '*******'); + $this->oLogger && $this->oLogger->AddSecret($sChallenge); + $this->sendRaw($sChallenge); $oResponse = $this->getResponse(); $SASL->verify($this->getResponseValue($oResponse)); */ @@ -154,13 +152,11 @@ class ManageSieveClient extends \MailSo\Net\NetClient else if ('PLAIN' === $type || 'OAUTHBEARER' === $type || 'XOAUTH2' === $type) { $sAuth = $SASL->authenticate($sLogin, $sPassword, $sLoginAuthKey); + $this->oLogger && $this->oLogger->AddSecret($sAuth); - if ($aCredentials['InitialAuthPlain']) - { + if ($aCredentials['InitialAuthPlain']) { $this->sendRaw("AUTHENTICATE \"{$type}\" \"{$sAuth}\""); - } - else - { + } else { $this->sendRaw("AUTHENTICATE \"{$type}\" {".\strlen($sAuth).'+}'); $this->sendRaw($sAuth); } @@ -174,6 +170,7 @@ class ManageSieveClient extends \MailSo\Net\NetClient { $sLogin = $SASL->authenticate($sLogin, $sPassword); $sPassword = $SASL->challenge(''); + $this->oLogger && $this->oLogger->AddSecret($sPassword); $this->sendRaw('AUTHENTICATE "LOGIN"'); $this->sendRaw('{'.\strlen($sLogin).'+}'); diff --git a/snappymail/v/0.0.0/app/libraries/MailSo/Smtp/SmtpClient.php b/snappymail/v/0.0.0/app/libraries/MailSo/Smtp/SmtpClient.php index 6a87de233..53d162ee6 100644 --- a/snappymail/v/0.0.0/app/libraries/MailSo/Smtp/SmtpClient.php +++ b/snappymail/v/0.0.0/app/libraries/MailSo/Smtp/SmtpClient.php @@ -185,19 +185,24 @@ class SmtpClient extends \MailSo\Net\NetClient if (0 === \strpos($type, 'SCRAM-')) { // RFC 5802 $sResult = $this->sendRequestWithCheck($SASL->authenticate($sLogin, $sPassword, $sResult), 234, ''); - $sResult = $this->sendRequestWithCheck($SASL->challenge($sResult), 235, '', true); - $SASL->verify($sResult); + $sChallenge = $SASL->challenge($sResult); + $this->oLogger && $this->oLogger->AddSecret($sChallenge); + $SASL->verify($this->sendRequestWithCheck($sChallenge, 235, '', true)); } else switch ($type) { // RFC 4616 case 'PLAIN': case 'XOAUTH2': case 'OAUTHBEARER': - $this->sendRequestWithCheck($SASL->authenticate($sLogin, $sPassword), 235, '', true); + $sAuth = $SASL->authenticate($sLogin, $sPassword); + $this->oLogger && $this->oLogger->AddSecret($sAuth); + $this->sendRequestWithCheck($sAuth, 235, '', true); break; case 'LOGIN': $sResult = $this->sendRequestWithCheck($SASL->authenticate($sLogin, $sPassword, $sResult), 334, ''); - $this->sendRequestWithCheck($SASL->challenge($sResult), 235, '', true); + $sPassword = $SASL->challenge($sResult); + $this->oLogger && $this->oLogger->AddSecret($sPassword); + $this->sendRequestWithCheck($sPassword, 235, '', true); break; // RFC 2195 @@ -342,7 +347,7 @@ class SmtpClient extends \MailSo\Net\NetClient $this->sendRequestWithCheck('DATA', 354); - $this->writeLog('Message data.', \LOG_INFO); + $this->writeLog('Message data.'); $this->bRunningCallback = true;