From e3c517f609d4ddc09acb90f2808762eb96d9428e Mon Sep 17 00:00:00 2001 From: Denis Chenu Date: Thu, 14 Jan 2021 19:14:17 +0100 Subject: [PATCH] Fixed issue #16853: Inconsistent filter behaviour when create token Fixed issue [security] #16884: LimeSurvey registration emails can be abused (thanks to winfried) Dev: remove flattenText in controller Dev: add some rules, leave LSYii_Validators Dev: use filter_var / FILTER_SANITIZE_STRING for attribute # Conflicts: # application/controllers/admin/tokens.php # Conflicts: # application/controllers/admin/tokens.php --- application/controllers/admin/tokens.php | 171 +++++++++++------------ application/models/Token.php | 34 ++++- application/models/TokenDynamic.php | 19 +-- 3 files changed, 116 insertions(+), 108 deletions(-) diff --git a/application/controllers/admin/tokens.php b/application/controllers/admin/tokens.php index 234d80ed015..4cc7af705b1 100644 --- a/application/controllers/admin/tokens.php +++ b/application/controllers/admin/tokens.php @@ -429,9 +429,9 @@ public function editMultiple() // Core Fields $aCoreTokenFields = array('firstname', 'lastname', 'emailstatus', 'token', 'language', 'sent', 'remindersent', 'remindercount', 'completed', 'usesleft'); foreach ($aCoreTokenFields as $sCoreTokenField) { - if (trim(Yii::app()->request->getPost($sCoreTokenField, 'lskeep')) != 'lskeep') { - $value = flattenText(Yii::app()->request->getPost($sCoreTokenField)); - if ($sCoreTokenField == 'language' and empty($value)){ + if (trim(App()->request->getPost($sCoreTokenField, 'lskeep')) != 'lskeep') { + $value = App()->request->getPost($sCoreTokenField); + if ($sCoreTokenField == 'language' && empty($value)) { continue; } $aData[$sCoreTokenField] = $value; @@ -442,7 +442,7 @@ public function editMultiple() $attrfieldnames = GetParticipantAttributes($iSurveyId); foreach ($attrfieldnames as $attr_name => $desc) { if (trim(Yii::app()->request->getPost($attr_name, 'lskeep')) != 'lskeep') { - $value = flattenText(Yii::app()->request->getPost($attr_name)); + $value = App()->request->getPost($attr_name); if ($desc['mandatory'] == 'Y' && trim($value) == '') { Yii::app()->setFlashMessage(sprintf(gT('%s cannot be left empty'), $desc['description']), 'error'); $this->getController()->refresh(); @@ -515,80 +515,77 @@ public function editToken($iSurveyId) $sOperation = Yii::app()->request->getPost('oper'); if (trim(Yii::app()->request->getPost('validfrom')) == '') { - $from = null; + $from = null; } else { - $from = date('Y-m-d H:i:s', strtotime(trim($_POST['validfrom']))); + $from = date('Y-m-d H:i:s', strtotime(trim($_POST['validfrom']))); } if (trim(Yii::app()->request->getPost('validuntil')) == '') { - $until = null; + $until = null; } else { - $until = date('Y-m-d H:i:s', strtotime(trim($_POST['validuntil']))); + $until = date('Y-m-d H:i:s', strtotime(trim($_POST['validuntil']))); } // if edit it will update the row if ($sOperation == 'edit' && Permission::model()->hasSurveyPermission($iSurveyId, 'tokens', 'update')) { - // if (Yii::app()->request->getPost('language') == '') - // { - // $sLang = Yii::app()->session['adminlang']; - // } - // else - // { - // $sLang = Yii::app()->request->getPost('language'); - // } - echo $from.','.$until; $aData = array( - 'firstname' => flattenText(Yii::app()->request->getPost('firstname')), - 'lastname' => flattenText(Yii::app()->request->getPost('lastname')), - 'email' => flattenText(Yii::app()->request->getPost('email')), - 'emailstatus' => flattenText(Yii::app()->request->getPost('emailstatus')), - 'token' => Token::sanitizeToken(Yii::app()->request->getPost('token')), - 'language' => flattenText(Yii::app()->request->getPost('language')), - 'sent' => flattenText(Yii::app()->request->getPost('sent')), - 'remindersent' => flattenText(Yii::app()->request->getPost('remindersent')), - 'remindercount' => flattenText(Yii::app()->request->getPost('remindercount')), - 'completed' => flattenText(Yii::app()->request->getPost('completed')), - 'usesleft' => flattenText(Yii::app()->request->getPost('usesleft')), - 'validfrom' => $from, - 'validuntil' => $until); + 'firstname' => App()()->request->getPost('firstname'), + 'lastname' => App()()->request->getPost('lastname'), + 'email' => App()()->request->getPost('email'), + 'emailstatus' => App()()->request->getPost('emailstatus'), + 'token' => App()()->request->getPost('token'), + 'language' => App()()->request->getPost('language'), + 'sent' => App()()->request->getPost('sent'), + 'remindersent' => App()()->request->getPost('remindersent'), + 'remindercount' => App()()->request->getPost('remindercount'), + 'completed' => App()()->request->getPost('completed'), + 'usesleft' => App()()->request->getPost('usesleft'), + 'validfrom' => $from, + 'validuntil' => $until + ); $attrfieldnames = getParticipantAttributes($iSurveyId); foreach ($attrfieldnames as $attr_name => $desc) { - $value = flattenText(Yii::app()->request->getPost($attr_name)); + $value = App()->request->getPost($attr_name); if ($desc['mandatory'] == 'Y' && trim($value) == '') { - Yii::app()->setFlashMessage(sprintf(gT('%s cannot be left empty'), $desc['description']), 'error'); + App()->setFlashMessage(sprintf(gT('%s cannot be left empty'), $desc['description']), 'error'); $this->getController()->refresh(); } $aData[$attr_name] = $value; } - $token = Token::model($iSurveyId)->find('tid='.Yii::app()->getRequest()->getPost('id')); + $oToken = Token::model($iSurveyId)->findByPk(App()->getRequest()->getPost('id')); + if (empty($oToken)) { + throw new CHttpException(400, sprintf(gT("Invalid token id"))); + } foreach ($aData as $k => $v) { - $token->$k = $v; + $oToken->$k = $v; } - echo $token->update(); + echo $oToken->update(); /* echo ? */ } // if add it will insert a new row elseif ($sOperation == 'add' && Permission::model()->hasSurveyPermission($iSurveyId, 'tokens', 'create')) { - if (Yii::app()->request->getPost('language') == '') { - $aData = array('firstname' => flattenText(Yii::app()->request->getPost('firstname')), - 'lastname' => flattenText(Yii::app()->request->getPost('lastname')), - 'email' => flattenText(Yii::app()->request->getPost('email')), - 'emailstatus' => flattenText(Yii::app()->request->getPost('emailstatus')), - 'token' => Token::sanitizeToken(Yii::app()->request->getPost('token')), - 'language' => flattenText(Yii::app()->request->getPost('language')), - 'sent' => flattenText(Yii::app()->request->getPost('sent')), - 'remindersent' => flattenText(Yii::app()->request->getPost('remindersent')), - 'remindercount' => flattenText(Yii::app()->request->getPost('remindercount')), - 'completed' => flattenText(Yii::app()->request->getPost('completed')), - 'usesleft' => flattenText(Yii::app()->request->getPost('usesleft')), - 'validfrom' => $from, - 'validuntil' => $until); + if (App()->request->getPost('language') == '') { + $aData = array( + 'firstname' => App()()->request->getPost('firstname'), + 'lastname' => App()()->request->getPost('lastname'), + 'email' => App()()->request->getPost('email'), + 'emailstatus' => App()()->request->getPost('emailstatus'), + 'token' => App()()->request->getPost('token'), + 'language' => App()()->request->getPost('language'), + 'sent' => App()()->request->getPost('sent'), + 'remindersent' => App()()->request->getPost('remindersent'), + 'remindercount' => App()()->request->getPost('remindercount'), + 'completed' => App()()->request->getPost('completed'), + 'usesleft' => App()()->request->getPost('usesleft'), + 'validfrom' => $from, + 'validuntil' => $until + ); } $attrfieldnames = Survey::model()->findByPk($iSurveyId)->tokenAttributes; foreach ($attrfieldnames as $attr_name => $desc) { - $value = flattenText(Yii::app()->request->getPost($attr_name)); + $value = App()->request->getPost($attr_name); if ($desc['mandatory'] == 'Y' && trim($value) == '') { Yii::app()->setFlashMessage(sprintf(gT('%s cannot be left empty'), $desc['description']), 'error'); $this->getController()->refresh(); @@ -672,16 +669,16 @@ public function addnew($iSurveyId) $sanitizedtoken = Token::sanitizeToken($request->getPost('token')); $aData = array( - 'firstname' => flattenText($request->getPost('firstname')), - 'lastname' => flattenText($request->getPost('lastname')), - 'email' => flattenText($request->getPost('email')), - 'emailstatus' => flattenText($request->getPost('emailstatus')), + 'firstname' => $request->getPost('firstname'), + 'lastname' => $request->getPost('lastname'), + 'email' => $request->getPost('email'), + 'emailstatus' => $request->getPost('emailstatus'), 'token' => $sanitizedtoken, 'language' => sanitize_languagecode($request->getPost('language')), - 'sent' => flattenText($request->getPost('sent')), - 'remindersent' => flattenText($request->getPost('remindersent')), - 'completed' => flattenText($request->getPost('completed')), - 'usesleft' => flattenText($request->getPost('usesleft')), + 'sent' => $request->getPost('sent'), + 'remindersent' => $request->getPost('remindersent'), + 'completed' => $request->getPost('completed'), + 'usesleft' => $request->getPost('usesleft'), 'validfrom' => $validfrom, 'validuntil' => $validuntil, ); @@ -702,20 +699,19 @@ public function addnew($iSurveyId) $aData[$attr_name] = App()->getRequest()->getPost($attr_name); } - $udresult = Token::model($iSurveyId)->findAll("token <> '' and token = '$sanitizedtoken'"); - if (count($udresult) == 0) { + if(!empty($sanitizedtoken) && Token::model($iSurveyId)->findByToken($sanitizedtoken)) { + $aData['success'] = false; + $aData['errors'] = array( + 'token' => array(gT("There is already an entry with that exact access code in the table. + The same access code cannot be used in multiple entries.")) + ); + } else { // AutoExecute $token = Token::create($iSurveyId); $token->setAttributes($aData, false); $inresult = $token->encryptSave(true); $aData['success'] = $inresult; $aData['errors'] = $token->getErrors(); - } else { - $aData['success'] = false; - $aData['errors'] = array( - 'token' => array(gT("There is already an entry with that exact access code in the table. - The same access code cannot be used in multiple entries.")) - ); } $aData['thissurvey'] = getSurveyInfo($iSurveyId); @@ -818,20 +814,19 @@ public function edit($iSurveyId, $iTokenId, $ajax = false) $_POST['remindersent'] = $datetimeobj->convert('Y-m-d H:i'); } - $aTokenData['firstname'] = flattenText($request->getPost('firstname')); - $aTokenData['lastname'] = flattenText($request->getPost('lastname')); - $aTokenData['email'] = flattenText($request->getPost('email')); - $aTokenData['emailstatus'] = flattenText($request->getPost('emailstatus')); - $sSanitizedToken = Token::sanitizeToken($request->getPost('token')); - $aTokenData['token'] = $sSanitizedToken; - $aTokenData['language'] = sanitize_languagecode($request->getPost('language')); - $aTokenData['sent'] = flattenText($request->getPost('sent')); - $aTokenData['completed'] = flattenText($request->getPost('completed')); - $aTokenData['usesleft'] = flattenText($request->getPost('usesleft')); + $aTokenData['firstname'] = $request->getPost('firstname'); + $aTokenData['lastname'] = $request->getPost('lastname'); + $aTokenData['email'] = $request->getPost('email'); + $aTokenData['emailstatus'] = $request->getPost('emailstatus'); + $aTokenData['token'] = $request->getPost('token'); + $aTokenData['language'] = $request->getPost('language'); + $aTokenData['sent'] = $request->getPost('sent'); + $aTokenData['completed'] = $request->getPost('completed'); + $aTokenData['usesleft'] = $request->getPost('usesleft'); $aTokenData['validfrom'] = $request->getPost('validfrom'); $aTokenData['validuntil'] = $request->getPost('validuntil'); - $aTokenData['remindersent'] = flattenText($request->getPost('remindersent')); - $aTokenData['remindercount'] = intval(flattenText($request->getPost('remindercount'))); + $aTokenData['remindersent'] = $request->getPost('remindersent'); + $aTokenData['remindercount'] = intval($request->getPost('remindercount')); $udresult = Token::model($iSurveyId)->findAll("tid <> :tid and token <> '' and token = :token", [':tid' => $iTokenId, ':token' => $sSanitizedToken]); $sOutput = ''; if (count($udresult) == 0) { @@ -956,23 +951,23 @@ public function addDummies($iSurveyId, $subaction = '') $aData['validuntil'] = $datetimeobj->convert('Y-m-d H:i:s'); } - $aData['firstname'] = flattenText(Yii::app()->request->getPost('firstname')); - $aData['lastname'] = flattenText(Yii::app()->request->getPost('lastname')); - $aData['email'] = flattenText(Yii::app()->request->getPost('email')); + $aData['firstname'] = App()()->request->getPost('firstname'); + $aData['lastname'] = App()()->request->getPost('lastname'); + $aData['email'] = App()()->request->getPost('email'); $aData['token'] = ''; - $aData['language'] = sanitize_languagecode(Yii::app()->request->getPost('language')); + $aData['language'] = sanitize_languagecode(App()()->request->getPost('language')); $aData['sent'] = 'N'; $aData['remindersent'] = 'N'; $aData['completed'] = 'N'; - $aData['usesleft'] = flattenText(Yii::app()->request->getPost('usesleft')); - $aData['amount'] = Yii::app()->request->getPost('amount'); - $aData['tokenlength'] = Yii::app()->request->getPost('tokenlen'); + $aData['usesleft'] = App()()->request->getPost('usesleft'); + $aData['amount'] = App()()->request->getPost('amount'); + $aData['tokenlength'] = App()()->request->getPost('tokenlen'); // add attributes $cntAttributeErrors = 0; $attrfieldnames = getTokenFieldsAndNames($iSurveyId, true); foreach ($attrfieldnames as $attr_name => $desc) { - $value = flattenText(Yii::app()->request->getPost($attr_name)); + $value = App()->request->getPost($attr_name); if ($desc['mandatory'] == 'Y' && trim($value) == '') { Yii::app()->setFlashMessage(sprintf(gT('%s cannot be left empty'), $desc['description']), 'error'); $cntAttributeErrors+=1; @@ -980,8 +975,8 @@ public function addDummies($iSurveyId, $subaction = '') $aData[$attr_name] = $value; } - $aData['amount'] = (int) Yii::app()->request->getPost('amount'); - $aData['tokenlength'] = (int) Yii::app()->request->getPost('tokenlen'); + $aData['amount'] = (int) App()->request->getPost('amount'); + $aData['tokenlength'] = (int) App()->request->getPost('tokenlen'); // Fill an array with all existing tokens $existingtokens = array(); diff --git a/application/models/Token.php b/application/models/Token.php index bb181c56ff5..26c13deda53 100644 --- a/application/models/Token.php +++ b/application/models/Token.php @@ -75,6 +75,13 @@ public function init() $this->completed = "N"; $this->emailstatus = "OK"; } + + /** @inheritdoc */ + public function primaryKey() + { + return 'tid'; + } + /** @inheritdoc */ public function attributeLabels() { @@ -307,6 +314,15 @@ public static function sanitizeToken($token) return preg_replace('/[^0-9a-zA-Z_~]/', '', $token); } + /** + * Sanitize string for any attribute + * @param string $attribute to sanitize + * @return string sanitized attribute + */ + public static function sanitizeAttribute($attribute) + { + return filter_var($attribute, FILTER_SANITIZE_STRING); + } /** * Generates a token for all token objects in this survey. @@ -396,9 +412,17 @@ public function rules() { $aRules = array( array('token', 'unique', 'allowEmpty' => true), - array('firstname', 'LSYii_Validators'), - array('lastname', 'LSYii_Validators'), + array('token', 'length', 'min' => 0, 'max'=>36), + array('token', 'filter', 'filter' => array(self::class, 'sanitizeToken')), + array('firstname', 'filter', 'filter' => array(self::class, 'sanitizeAttribute')), + array('lastname', 'filter', 'filter' => array(self::class, 'sanitizeAttribute')), + array('language', 'LSYii_Validators', 'isLanguage'=>true), array(implode(',', $this->tableSchema->columnNames), 'safe'), + /* pseudo date : force date or specific string ? */ + array('remindersent', 'length', 'min' => 0, 'max'=>17), + array('remindersent', 'filter', 'filter' => array(self::class, 'sanitizeAttribute')), + array('completed', 'length', 'min' => 0, 'max'=>17), + array('remindersent', 'filter', 'filter' => array(self::class, 'sanitizeAttribute')), array('remindercount', 'numerical', 'integerOnly'=>true, 'allowEmpty'=>true), array('email', 'filter', 'filter'=>'trim'), array('email', 'LSYii_EmailIDNAValidator', 'allowEmpty'=>true, 'allowMultiple'=>true, 'except'=>'allowinvalidemail'), @@ -410,7 +434,11 @@ public function rules() array('emailstatus', 'default', 'value' => 'OK'), ); foreach (decodeTokenAttributes($this->survey->attributedescriptions) as $key => $info) { - $aRules[] = array($key, 'LSYii_Validators', 'except'=>'FinalSubmit'); + $aRules[] = array( + $key, 'filter', + 'filter' => array(self::class, 'sanitizeAttribute'), + 'except'=>'FinalSubmit' + ); } return $aRules; } diff --git a/application/models/TokenDynamic.php b/application/models/TokenDynamic.php index d2eedf8c26b..56763ec8a4e 100644 --- a/application/models/TokenDynamic.php +++ b/application/models/TokenDynamic.php @@ -102,23 +102,8 @@ public function primaryKey() **/ public function rules() { - $aRules = array( - array('token', 'unique', 'allowEmpty' => true), - array('firstname', 'LSYii_Validators'), - array('lastname', 'LSYii_Validators'), - array(implode(',', $this->tableSchema->columnNames), 'safe'), - array('remindercount', 'numerical', 'integerOnly'=>true, 'allowEmpty'=>true), - array('email', 'filter', 'filter'=>'trim'), - array('email', 'LSYii_EmailIDNAValidator', 'allowEmpty'=>true, 'allowMultiple'=>true, 'except'=>'allowinvalidemail'), - array('usesleft', 'numerical', 'integerOnly'=>true, 'allowEmpty'=>true, 'min'=>-2147483647, 'max'=>2147483647), - array('mpid', 'numerical', 'integerOnly'=>true, 'allowEmpty'=>true), - array('blacklisted', 'in', 'range'=>array('Y', 'N'), 'allowEmpty'=>true), - array('emailstatus', 'default', 'value' => 'OK'), - ); - foreach (decodeTokenAttributes($this->survey->attributedescriptions) as $key => $info) { - $aRules[] = array($key, 'LSYii_Validators', 'except'=>'FinalSubmit'); - } - return $aRules; + $Token = Token::model(self::$sid); + return $Token->rules(); } /** @inheritdoc */