From b17e5dd11dcefcdc01be1d8267f912101cecdf61 Mon Sep 17 00:00:00 2001 From: rolfen Date: Sat, 20 Oct 2018 12:35:57 +0000 Subject: [PATCH 1/3] try to plug the hole in the custom sql plugin --- .../ChangePasswordCustomSqlDriver.php | 46 +++++++++++++++---- 1 file changed, 37 insertions(+), 9 deletions(-) diff --git a/plugins/change-password-custom-sql/ChangePasswordCustomSqlDriver.php b/plugins/change-password-custom-sql/ChangePasswordCustomSqlDriver.php index c0dd532b49..33f79c279f 100644 --- a/plugins/change-password-custom-sql/ChangePasswordCustomSqlDriver.php +++ b/plugins/change-password-custom-sql/ChangePasswordCustomSqlDriver.php @@ -146,7 +146,7 @@ public function ChangePassword(\RainLoop\Account $oAccount, $sPrevPassword, $sNe $dsn = 'mysql:host='.$this->mHost.';dbname='.$this->mDatabase.';charset=utf8'; $options = array( - PDO::ATTR_EMULATE_PREPARES => false, + PDO::ATTR_EMULATE_PREPARES => true, PDO::ATTR_PERSISTENT => true, PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION ); @@ -160,15 +160,43 @@ public function ChangePassword(\RainLoop\Account $oAccount, $sPrevPassword, $sNe $sEmailUser = \MailSo\Base\Utils::GetAccountNameFromEmail($sEmail); $sEmailDomain = \MailSo\Base\Utils::GetDomainFromEmail($sEmail); - //simple check + // some variables cannot be prepared + /* + $this->mSql = str_replace(array( + ':table' + ), array( + $this->mTable + ), $this->mSql); + */ + + $placeholders = array( + ':table' => $this->mTable, + ':email' => $sEmail, + ':oldpass' => $sPrevPassword, + ':newpass' => $sNewPassword, + ':domain' => $sEmailDomain, + ':username' => $sEmailUser + ); + + $statement = $conn->prepare($this->mSql); + + $this->oLogger->Write($this->mSql); + + $statement->bindValue(':table', 'accounts'); + $statement->bindValue(':email', $sEmail); + $statement->bindValue(':oldpass', $sPrevPassword); + $statement->bindValue(':newpass', $sNewPassword); + $statement->bindValue(':domain', $sEmailDomain); + $statement->bindValue(':username', $sEmailUser); + + $mSqlReturn = $statement->execute(); + + ob_start(); + $statement->debugDumpParams(); + $r = ob_get_contents(); + ob_end_clean(); + $this->oLogger->Write($r); - $old = array(':email', ':oldpass', ':newpass', ':domain', ':username', ':table' ); - $new = array($sEmail, $sPrevPassword, $sNewPassword, $sEmailDomain, $sEmailUser, $this->mTable); - - $this->mSql = str_replace($old, $new, $this->mSql); - - $update = $conn->prepare($this->mSql); - $mSqlReturn = $update->execute(array()); if ($mSqlReturn == true) { $bResult = true; From 353964e72988b8cf1d0c5735415406969a6d1d56 Mon Sep 17 00:00:00 2001 From: rolfen Date: Sat, 20 Oct 2018 15:13:05 +0000 Subject: [PATCH 2/3] working version --- .../ChangePasswordCustomSqlDriver.php | 31 ++++++++++++------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/plugins/change-password-custom-sql/ChangePasswordCustomSqlDriver.php b/plugins/change-password-custom-sql/ChangePasswordCustomSqlDriver.php index 33f79c279f..1a60346e2f 100644 --- a/plugins/change-password-custom-sql/ChangePasswordCustomSqlDriver.php +++ b/plugins/change-password-custom-sql/ChangePasswordCustomSqlDriver.php @@ -161,16 +161,13 @@ public function ChangePassword(\RainLoop\Account $oAccount, $sPrevPassword, $sNe $sEmailDomain = \MailSo\Base\Utils::GetDomainFromEmail($sEmail); // some variables cannot be prepared - /* $this->mSql = str_replace(array( ':table' ), array( $this->mTable ), $this->mSql); - */ $placeholders = array( - ':table' => $this->mTable, ':email' => $sEmail, ':oldpass' => $sPrevPassword, ':newpass' => $sNewPassword, @@ -178,24 +175,36 @@ public function ChangePassword(\RainLoop\Account $oAccount, $sPrevPassword, $sNe ':username' => $sEmailUser ); - $statement = $conn->prepare($this->mSql); + // we have to check that all placehoders are used in the query, passing any unused placeholders will generate an error + $used_placeholders = array(); + + foreach($placeholders as $placeholder => $value) { + if(preg_match_all('/'.$placeholder . '(?![a-zA-Z0-9\-])'.'/', $this->mSql) === 1) { + // backwards-compabitibility: remove single and double quotes around placeholders + $this->mSql = str_replace('`'.$placeholder.'`', $placeholder, $this->mSql); + $this->mSql = str_replace("'".$placeholder."'", $placeholder, $this->mSql); + $this->mSql = str_replace('"'.$placeholder.'"', $placeholder, $this->mSql); + $used_placeholders[$placeholder] = $value; + } + } - $this->oLogger->Write($this->mSql); + $statement = $conn->prepare($this->mSql); - $statement->bindValue(':table', 'accounts'); - $statement->bindValue(':email', $sEmail); - $statement->bindValue(':oldpass', $sPrevPassword); - $statement->bindValue(':newpass', $sNewPassword); - $statement->bindValue(':domain', $sEmailDomain); - $statement->bindValue(':username', $sEmailUser); + // everything is ready (hopefully), bind the values + foreach($used_placeholders as $placeholder => $value) { + $statement->bindValue($placeholder, $value); + } + // and execute $mSqlReturn = $statement->execute(); + /* can be used for debugging ob_start(); $statement->debugDumpParams(); $r = ob_get_contents(); ob_end_clean(); $this->oLogger->Write($r); + */ if ($mSqlReturn == true) { From 32ccf8c76fa5805466f9118aae46f944ffb0bb08 Mon Sep 17 00:00:00 2001 From: rolfen Date: Sat, 20 Oct 2018 15:40:11 +0000 Subject: [PATCH 3/3] improve description --- plugins/change-password-custom-sql/index.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/change-password-custom-sql/index.php b/plugins/change-password-custom-sql/index.php index ad7f7e222c..692f86e7e5 100644 --- a/plugins/change-password-custom-sql/index.php +++ b/plugins/change-password-custom-sql/index.php @@ -46,7 +46,7 @@ public function configMapping() \RainLoop\Plugins\Property::NewInstance('mTable')->SetLabel('MySQL Table'), \RainLoop\Plugins\Property::NewInstance('mSql')->SetLabel('SQL statement') ->SetType(\RainLoop\Enumerations\PluginPropertyType::STRING_TEXT) - ->SetDescription('SQL statement (allowed wildcards :table, :email, :oldpass, :newpass, :domain, :username). When using MD5 OR SHA1 at tables, write it directly here as SQL functions. Fro non-SQL encryptions use another plugin or wait for new version.') + ->SetDescription('SQL statement (allowed wildcards :table, :email, :oldpass, :newpass, :domain, :username). Use SQL functions for encryption.') ->SetDefaultValue('UPDATE :table SET password = md5(:newpass) WHERE domain = :domain AND username = :username and oldpass = md5(:oldpass)') ); }