Skip to content

Commit

Permalink
Fixed issue #16910: Any plugin public function can be done without an…
Browse files Browse the repository at this point in the history
…y control on rights (#1706)

Dev: allowedPublicMethods to null by default, used only for compatible plugin
Dev: add Permission checker on getPluginSettings and saveSettings
  • Loading branch information
Shnoulle committed Jan 15, 2021
1 parent 7ae3274 commit aa28736
Show file tree
Hide file tree
Showing 18 changed files with 90 additions and 36 deletions.
5 changes: 4 additions & 1 deletion application/controllers/admin/PluginHelper.php
Expand Up @@ -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);
Expand Down
3 changes: 3 additions & 0 deletions application/core/plugins/AuditLog/AuditLog.php
Expand Up @@ -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',
Expand Down
3 changes: 3 additions & 0 deletions application/core/plugins/AuthLDAP/AuthLDAP.php
Expand Up @@ -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.
Expand Down
3 changes: 3 additions & 0 deletions application/core/plugins/Authdb/Authdb.php
Expand Up @@ -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()
{
/**
Expand Down
5 changes: 4 additions & 1 deletion application/core/plugins/Authwebserver/Authwebserver.php
Expand Up @@ -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',
Expand Down
Expand Up @@ -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',
Expand Down
10 changes: 6 additions & 4 deletions application/core/plugins/ExportR/ExportR.php
Expand Up @@ -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
*/
Expand Down Expand Up @@ -78,4 +80,4 @@ public function newExport()

$event->set('writer', $writer);
}
}
}
7 changes: 5 additions & 2 deletions application/core/plugins/ExportSTATAxml/ExportSTATAxml.php
Expand Up @@ -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()
{

Expand Down Expand Up @@ -79,4 +82,4 @@ public function newExport()
$writer = new STATAxmlWriter($pluginsettings);
$event->set('writer', $writer);
}
}
}
Expand Up @@ -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',
Expand Down Expand Up @@ -145,4 +148,4 @@ private function getRandomString($length=8, $uppercase=false, $numeric=false, $n

return $str;
}
}
}
5 changes: 4 additions & 1 deletion application/core/plugins/UpdateCheck/UpdateCheck.php
Expand Up @@ -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
*/
Expand Down Expand Up @@ -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'];

This comment has been minimized.

Copy link
@olleharstedt

olleharstedt Jan 15, 2021

Collaborator

This breaks the notification menu. It always includes <script> now, which breaks the JSON.

This comment has been minimized.

Copy link
@olleharstedt

olleharstedt Jan 15, 2021

Collaborator

Fixed. Why was this set to true? oO

This comment has been minimized.

Copy link
@Shnoulle

Shnoulle Jan 16, 2021

Author Collaborator

Wy i set it to true ???

This comment has been minimized.

Copy link
@Shnoulle

Shnoulle Jan 16, 2021

Author Collaborator

This breaks the notification menu. It always includes <script> now, which breaks the JSON.

then if Yii::app()->session['do_extensions_update_check']; is true : it broke menu ?


if ($controller == 'admin' && $doUpdateCheckFlag) {

Expand Down
3 changes: 3 additions & 0 deletions application/core/plugins/customToken/customToken.php
Expand Up @@ -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()
{
/**
Expand Down
Expand Up @@ -3,9 +3,9 @@
* expressionFixedDbVar : add some fixed DB var : SEED, STARTDATE …
*
* @author Denis Chenu <denis@sondages.pro>
* @copyright 2019 Denis Chenu <http://www.sondages.pro>
* @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
Expand All @@ -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(
Expand Down
Expand Up @@ -4,9 +4,9 @@
* This don't manage subquestion Scale Y or Scale X
*
* @author Denis Chenu <denis@sondages.pro>
* @copyright 2019 Denis Chenu <http://www.sondages.pro>
* @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
Expand All @@ -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');
Expand Down
Expand Up @@ -3,9 +3,9 @@
* expressionQuestionHelp : add QCODE.help for expression Manager
*
* @author Denis Chenu <denis@sondages.pro>
* @copyright 2019 Denis Chenu <http://www.sondages.pro>
* @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
Expand All @@ -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');
Expand Down
Expand Up @@ -4,9 +4,9 @@
* Needed for some smtp server, see mantis issue #10529 <https://bugs.limesurvey.org/view.php?id=10529>
*
* @author Denis Chenu <denis@sondages.pro>
* @copyright 2019 Denis Chenu <http://www.sondages.pro>
* @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
Expand All @@ -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');
Expand Down
18 changes: 8 additions & 10 deletions application/core/plugins/oldUrlCompat/oldUrlCompat.php
Expand Up @@ -3,9 +3,9 @@
* Plugin to redirect old url system (index.php?sid=surveyid) to the new url
*
* @author Denis Chenu <denis@sondages.pro>
* @copyright 2016 LimeSurvey team <https://www.limesurvey.org>
* @copyright 2016-2020 LimeSurvey team <https://www.limesurvey.org>
* @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
Expand All @@ -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
Expand Down
5 changes: 4 additions & 1 deletion application/core/plugins/statFunctions/statFunctions.php
@@ -1,7 +1,7 @@
<?php
/**
* @author Denis Chenu <denis@sondages.pro>
* @copyright 2019 Denis Chenu <http://www.sondages.pro>
* @copyright 2019 LimeSurvey - Denis Chenu
* @license GPL version 3
* @version 0.1.1
*
Expand All @@ -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');
Expand Down
16 changes: 14 additions & 2 deletions application/libraries/PluginManager/PluginBase.php
Expand Up @@ -43,7 +43,8 @@ abstract class PluginBase implements iPlugin
private $store = null;

/**
* @var array
* Global settings of plugin
* @var array[]
*/
protected $settings = [];

Expand All @@ -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
Expand Down Expand Up @@ -161,7 +168,9 @@ public function getId()
*/
public function getPluginSettings($getValues = true)
{

if(!Permission::model()->hasGlobalPermission('settings','read')) {

This comment has been minimized.

Copy link
@c-schmitz

c-schmitz Feb 16, 2021

Contributor

Here...

This comment has been minimized.

Copy link
@olleharstedt

olleharstedt Feb 17, 2021

Collaborator

Here what? 😆

throw new CHttpException(403);
}
$settings = $this->settings;
foreach ($settings as $name => &$setting) {
if ($getValues) {
Expand Down Expand Up @@ -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);
}
Expand Down

0 comments on commit aa28736

Please sign in to comment.