From aa287360a6f171009e0df44999a2da30eea71eb9 Mon Sep 17 00:00:00 2001 From: Denis Chenu Date: Fri, 15 Jan 2021 12:57:49 +0100 Subject: [PATCH] Fixed issue #16910: Any plugin public function can be done without any control on rights (#1706) Dev: allowedPublicMethods to null by default, used only for compatible plugin Dev: add Permission checker on getPluginSettings and saveSettings --- application/controllers/admin/PluginHelper.php | 5 ++++- application/core/plugins/AuditLog/AuditLog.php | 3 +++ application/core/plugins/AuthLDAP/AuthLDAP.php | 3 +++ application/core/plugins/Authdb/Authdb.php | 3 +++ .../plugins/Authwebserver/Authwebserver.php | 5 ++++- .../ComfortUpdateChecker.php | 6 ++++-- application/core/plugins/ExportR/ExportR.php | 10 ++++++---- .../plugins/ExportSTATAxml/ExportSTATAxml.php | 7 +++++-- .../PasswordRequirement.php | 5 ++++- .../core/plugins/UpdateCheck/UpdateCheck.php | 5 ++++- .../core/plugins/customToken/customToken.php | 3 +++ .../expressionFixedDbVar.php | 9 ++++++--- .../expressionQuestionForAll.php | 7 +++++-- .../expressionQuestionHelp.php | 8 +++++--- .../mailSenderToFrom/mailSenderToFrom.php | 8 +++++--- .../core/plugins/oldUrlCompat/oldUrlCompat.php | 18 ++++++++---------- .../plugins/statFunctions/statFunctions.php | 5 ++++- .../libraries/PluginManager/PluginBase.php | 16 ++++++++++++++-- 18 files changed, 90 insertions(+), 36 deletions(-) diff --git a/application/controllers/admin/PluginHelper.php b/application/controllers/admin/PluginHelper.php index c15cc45ac8d..46928b0e19a 100644 --- a/application/controllers/admin/PluginHelper.php +++ b/application/controllers/admin/PluginHelper.php @@ -97,9 +97,12 @@ protected function getPluginInstanceAndMethod($pluginName, $methodName) $pluginManager = App()->getPluginManager(); $pluginInstance = $refClass->newInstance($pluginManager, $record->id); + /* Check if method is in allowed list */ + if(is_array($pluginInstance->allowedPublicMethods) && !in_array($methodName,$pluginInstance->allowedPublicMethods)) { + throw new \CHttpException(400, sprintf(gT("Forbidden call of method %s for plugin %s"),$methodName,$pluginName)); + } Yii::app()->setPlugin($pluginInstance); - // Get plugin method, abort if not found try { $refMethod = $refClass->getMethod($methodName); diff --git a/application/core/plugins/AuditLog/AuditLog.php b/application/core/plugins/AuditLog/AuditLog.php index a2d4f72525a..477a5cc6e0b 100644 --- a/application/core/plugins/AuditLog/AuditLog.php +++ b/application/core/plugins/AuditLog/AuditLog.php @@ -5,6 +5,9 @@ class AuditLog extends \LimeSurvey\PluginManager\PluginBase { static protected $description = 'Core: Create an audit log of changes'; static protected $name = 'auditlog'; + /** @inheritdoc, this plugin didn't have any public method */ + public $allowedPublicMethods = array(); + protected $settings = array( 'AuditLog_Log_UserSave' => array( 'type' => 'checkbox', diff --git a/application/core/plugins/AuthLDAP/AuthLDAP.php b/application/core/plugins/AuthLDAP/AuthLDAP.php index d89737de294..bd1a362920c 100644 --- a/application/core/plugins/AuthLDAP/AuthLDAP.php +++ b/application/core/plugins/AuthLDAP/AuthLDAP.php @@ -6,6 +6,9 @@ class AuthLDAP extends LimeSurvey\PluginManager\AuthPluginBase static protected $description = 'Core: LDAP authentication'; static protected $name = 'LDAP'; + /** @inheritdoc, this plugin didn't have any public method */ + public $allowedPublicMethods = array(); + /** * Can we autocreate users? For the moment this is disabled, will be moved * to a setting when we have more robust user creation system. diff --git a/application/core/plugins/Authdb/Authdb.php b/application/core/plugins/Authdb/Authdb.php index 9ff61f46b15..b7c33d22d79 100644 --- a/application/core/plugins/Authdb/Authdb.php +++ b/application/core/plugins/Authdb/Authdb.php @@ -7,6 +7,9 @@ class Authdb extends AuthPluginBase static protected $description = 'Core: Database authentication + exports'; static protected $name = 'LimeSurvey internal database'; + /** @inheritdoc, this plugin didn't have any public method */ + public $allowedPublicMethods = array(); + public function init() { /** diff --git a/application/core/plugins/Authwebserver/Authwebserver.php b/application/core/plugins/Authwebserver/Authwebserver.php index 6e6c1f95022..63b7cb34009 100644 --- a/application/core/plugins/Authwebserver/Authwebserver.php +++ b/application/core/plugins/Authwebserver/Authwebserver.php @@ -5,7 +5,10 @@ class Authwebserver extends LimeSurvey\PluginManager\AuthPluginBase static protected $description = 'Core: Webserver authentication'; static protected $name = 'Webserver'; - + + /** @inheritdoc, this plugin didn't have any public method */ + public $allowedPublicMethods = array(); + protected $settings = array( 'strip_domain' => array( 'type' => 'checkbox', diff --git a/application/core/plugins/ComfortUpdateChecker/ComfortUpdateChecker.php b/application/core/plugins/ComfortUpdateChecker/ComfortUpdateChecker.php index 97d7820611e..4b8c314f366 100644 --- a/application/core/plugins/ComfortUpdateChecker/ComfortUpdateChecker.php +++ b/application/core/plugins/ComfortUpdateChecker/ComfortUpdateChecker.php @@ -17,9 +17,11 @@ class ComfortUpdateChecker extends PluginBase static protected $description = 'Update Checker for Comfort Update users'; static protected $name = 'ComfortUpdateChecker'; - - protected $settings = [ + /** @inheritdoc, this plugin didn't have any public method */ + public $allowedPublicMethods = array(); + + protected $settings = [ 'only_security_update' => array( 'type' => 'checkbox', 'label' => 'Notification only for security updates', diff --git a/application/core/plugins/ExportR/ExportR.php b/application/core/plugins/ExportR/ExportR.php index d3fc7bd5017..a36198295ec 100644 --- a/application/core/plugins/ExportR/ExportR.php +++ b/application/core/plugins/ExportR/ExportR.php @@ -3,13 +3,15 @@ class ExportR extends \LimeSurvey\PluginManager\PluginBase { protected $storage = 'DbStorage'; - + static protected $description = 'Core: R-export'; static protected $name = 'Export results to R'; - + + /** @inheritdoc, this plugin didn't have any public method */ + public $allowedPublicMethods = array(); + public function init() { - /** * Here you should handle subscribing to the events your plugin will handle */ @@ -78,4 +80,4 @@ public function newExport() $event->set('writer', $writer); } -} \ No newline at end of file +} diff --git a/application/core/plugins/ExportSTATAxml/ExportSTATAxml.php b/application/core/plugins/ExportSTATAxml/ExportSTATAxml.php index 2517dd8fafe..96bf0c90a4b 100644 --- a/application/core/plugins/ExportSTATAxml/ExportSTATAxml.php +++ b/application/core/plugins/ExportSTATAxml/ExportSTATAxml.php @@ -6,7 +6,10 @@ class ExportSTATAxml extends \LimeSurvey\PluginManager\PluginBase static protected $description = 'Core: Export survey results to a STATA xml file'; static protected $name = 'STATA Export'; - + + /** @inheritdoc, this plugin didn't have any public method */ + public $allowedPublicMethods = array(); + public function init() { @@ -79,4 +82,4 @@ public function newExport() $writer = new STATAxmlWriter($pluginsettings); $event->set('writer', $writer); } -} \ No newline at end of file +} diff --git a/application/core/plugins/PasswordRequirement/PasswordRequirement.php b/application/core/plugins/PasswordRequirement/PasswordRequirement.php index 00a46a94b28..f5dbf0ec261 100644 --- a/application/core/plugins/PasswordRequirement/PasswordRequirement.php +++ b/application/core/plugins/PasswordRequirement/PasswordRequirement.php @@ -7,6 +7,9 @@ class PasswordRequirement extends \LimeSurvey\PluginManager\PluginBase { */ protected $storage = 'DbStorage'; + /** @inheritdoc, this plugin didn't have any public method */ + public $allowedPublicMethods = array(); + protected $settings = [ 'needsNumber' => array( 'label' => 'Require at least one digit', @@ -145,4 +148,4 @@ private function getRandomString($length=8, $uppercase=false, $numeric=false, $n return $str; } -} \ No newline at end of file +} diff --git a/application/core/plugins/UpdateCheck/UpdateCheck.php b/application/core/plugins/UpdateCheck/UpdateCheck.php index 3387ce4738e..0f6b9d7501b 100644 --- a/application/core/plugins/UpdateCheck/UpdateCheck.php +++ b/application/core/plugins/UpdateCheck/UpdateCheck.php @@ -30,6 +30,9 @@ class UpdateCheck extends PluginBase */ protected $storage = 'DbStorage'; + /** @inheritdoc, this plugin didn't have any public method */ + public $allowedPublicMethods = array('checkAll'); + /** * @return void */ @@ -66,7 +69,7 @@ public function afterSuccessfulLogin() public function beforeControllerAction() { $controller = $this->getEvent()->get('controller'); - $doUpdateCheckFlag = Yii::app()->session['do_extensions_update_check']; + $doUpdateCheckFlag = true; //Yii::app()->session['do_extensions_update_check']; if ($controller == 'admin' && $doUpdateCheckFlag) { diff --git a/application/core/plugins/customToken/customToken.php b/application/core/plugins/customToken/customToken.php index 2e44dfef343..a45d2a7544d 100644 --- a/application/core/plugins/customToken/customToken.php +++ b/application/core/plugins/customToken/customToken.php @@ -5,6 +5,9 @@ class customToken extends PluginBase { static protected $name = 'customToken'; static protected $description = 'At token generation this plugin enforces certain token formats like Numeric, non-ambiguous or uppercase tokens'; + /** @inheritdoc, this plugin didn't have any public method */ + public $allowedPublicMethods = array(); + public function init() { /** diff --git a/application/core/plugins/expressionFixedDbVar/expressionFixedDbVar.php b/application/core/plugins/expressionFixedDbVar/expressionFixedDbVar.php index 803a82f54ce..4d4df57c7a7 100644 --- a/application/core/plugins/expressionFixedDbVar/expressionFixedDbVar.php +++ b/application/core/plugins/expressionFixedDbVar/expressionFixedDbVar.php @@ -3,9 +3,9 @@ * expressionFixedDbVar : add some fixed DB var : SEED, STARTDATE … * * @author Denis Chenu - * @copyright 2019 Denis Chenu + * @copyright 2019 LimeSurvey - Denis Chenu * @license GPL version 3 - * @version 1.0.0 + * @version 1.0.1 * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -26,8 +26,11 @@ class expressionFixedDbVar extends PluginBase static protected $description = 'Add SEED and other DB var in ExpressionScript Engine.'; static protected $name = 'expressionFixedDbVar'; + /** @inheritdoc, this plugin didn't have any public method */ + public $allowedPublicMethods = array(); + /** - * @var array[] the settings + * @inheritdoc */ protected $settings = array( 'SEED' => array( diff --git a/application/core/plugins/expressionQuestionForAll/expressionQuestionForAll.php b/application/core/plugins/expressionQuestionForAll/expressionQuestionForAll.php index 823754637ee..c253dcdd794 100644 --- a/application/core/plugins/expressionQuestionForAll/expressionQuestionForAll.php +++ b/application/core/plugins/expressionQuestionForAll/expressionQuestionForAll.php @@ -4,9 +4,9 @@ * This don't manage subquestion Scale Y or Scale X * * @author Denis Chenu - * @copyright 2019 Denis Chenu + * @copyright 2019 LimeSurvey - Denis Chenu * @license GPL version 3 - * @version 1.0.0 + * @version 1.0.1 * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -26,6 +26,9 @@ class expressionQuestionForAll extends PluginBase static protected $description = 'Add QCODE.question for question with subquestion for expression Manager.'; static protected $name = 'expressionQuestionForAll'; + /** @inheritdoc, this plugin didn't have any public method */ + public $allowedPublicMethods = array(); + public function init() { $this->subscribe('setVariableExpressionEnd','addQuestionAll'); diff --git a/application/core/plugins/expressionQuestionHelp/expressionQuestionHelp.php b/application/core/plugins/expressionQuestionHelp/expressionQuestionHelp.php index 21c097421d7..8a0a144c37a 100644 --- a/application/core/plugins/expressionQuestionHelp/expressionQuestionHelp.php +++ b/application/core/plugins/expressionQuestionHelp/expressionQuestionHelp.php @@ -3,9 +3,9 @@ * expressionQuestionHelp : add QCODE.help for expression Manager * * @author Denis Chenu - * @copyright 2019 Denis Chenu + * @copyright 2019 LimeSurvey - Denis Chenu * @license GPL version 3 - * @version 1.0.0 + * @version 1.0.1 * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -25,7 +25,9 @@ class expressionQuestionHelp extends PluginBase static protected $description = 'Add .help to properties of questions.'; static protected $name = 'expressionQuestionHelp'; - + /** @inheritdoc, this plugin didn't have any public method */ + public $allowedPublicMethods = array(); + public function init() { $this->subscribe('setVariableExpressionEnd','addQuestionHelp'); diff --git a/application/core/plugins/mailSenderToFrom/mailSenderToFrom.php b/application/core/plugins/mailSenderToFrom/mailSenderToFrom.php index 33c960823da..6f11d9c2d74 100644 --- a/application/core/plugins/mailSenderToFrom/mailSenderToFrom.php +++ b/application/core/plugins/mailSenderToFrom/mailSenderToFrom.php @@ -4,9 +4,9 @@ * Needed for some smtp server, see mantis issue #10529 * * @author Denis Chenu - * @copyright 2019 Denis Chenu + * @copyright 2019 LimeSurvey - Denis Chenu * @license MIT - * @version 1.0.0 + * @version 1.0.1 * * This program is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY; without even the implied warranty of @@ -18,7 +18,9 @@ class mailSenderToFrom extends PluginBase static protected $description = 'Set sender to the SMTP user.'; static protected $name = 'mailSenderToFrom'; - + /** @inheritdoc, this plugin didn't have any public method */ + public $allowedPublicMethods = array(); + public function init() { $this->subscribe('beforeEmail','beforeEmail'); diff --git a/application/core/plugins/oldUrlCompat/oldUrlCompat.php b/application/core/plugins/oldUrlCompat/oldUrlCompat.php index 537cfd3afce..cead7838801 100644 --- a/application/core/plugins/oldUrlCompat/oldUrlCompat.php +++ b/application/core/plugins/oldUrlCompat/oldUrlCompat.php @@ -3,9 +3,9 @@ * Plugin to redirect old url system (index.php?sid=surveyid) to the new url * * @author Denis Chenu - * @copyright 2016 LimeSurvey team + * @copyright 2016-2020 LimeSurvey team * @license GPL v3 - * @version 0.0.1 + * @version 0.1.0 * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -22,16 +22,14 @@ class oldUrlCompat extends PluginBase static protected $name = 'oldUrlCompat'; static protected $description = 'Old url (pre-2.0) compatible system'; - /** init broke plugin management */ - //~ public function init() - //~ { - //~ $this->subscribe('afterPluginLoad','oldUrlCompat'); - //~ } - public function __construct(\LimeSurvey\PluginManager\PluginManager $manager, $id) + /** @inheritdoc, this plugin didn't have any public method */ + public $allowedPublicMethods = array(); + + public function init() { - parent::__construct($manager, $id); - $this->subscribe('afterPluginLoad', 'oldUrlCompat'); + $this->subscribe('afterPluginLoad','oldUrlCompat'); } + /** * Forward survey controller if we are in default controller and a sid GET parameters is set * @return void diff --git a/application/core/plugins/statFunctions/statFunctions.php b/application/core/plugins/statFunctions/statFunctions.php index 7adb1000cfa..f678b550ffa 100644 --- a/application/core/plugins/statFunctions/statFunctions.php +++ b/application/core/plugins/statFunctions/statFunctions.php @@ -1,7 +1,7 @@ - * @copyright 2019 Denis Chenu + * @copyright 2019 LimeSurvey - Denis Chenu * @license GPL version 3 * @version 0.1.1 * @@ -23,6 +23,9 @@ class statFunctions extends PluginBase protected static $description = 'Add some function in ExpressionScript Engine to get count from other responses'; protected static $name = 'statCountFunctions'; + /** @inheritdoc, this plugin didn't have any public method */ + public $allowedPublicMethods = array(); + public function init() { $this->subscribe('ExpressionManagerStart', 'newValidFunctions'); diff --git a/application/libraries/PluginManager/PluginBase.php b/application/libraries/PluginManager/PluginBase.php index 19862587760..32279bb21bc 100644 --- a/application/libraries/PluginManager/PluginBase.php +++ b/application/libraries/PluginManager/PluginBase.php @@ -43,7 +43,8 @@ abstract class PluginBase implements iPlugin private $store = null; /** - * @var array + * Global settings of plugin + * @var array[] */ protected $settings = []; @@ -60,6 +61,12 @@ abstract class PluginBase implements iPlugin */ public $config = null; + /** + * List of allowed public method, null mean all metod are allowed. Else method must be in the list + * @var string[]|null + */ + public $allowedPublicMethods = null; + /** * Constructor for the plugin * @todo Add proper type hint in 3.0 @@ -161,7 +168,9 @@ public function getId() */ public function getPluginSettings($getValues = true) { - + if(!Permission::model()->hasGlobalPermission('settings','read')) { + throw new CHttpException(403); + } $settings = $this->settings; foreach ($settings as $name => &$setting) { if ($getValues) { @@ -247,6 +256,9 @@ protected function registerSetting($name, $options = array('type' => 'string')) */ public function saveSettings($settings) { + if(!Permission::model()->hasGlobalPermission('settings','update')) { + throw new CHttpException(403); + } foreach ($settings as $name => $setting) { $this->set($name, $setting); }