Skip to content

Commit

Permalink
Close #21: Don't import arbitrary globals from the query string
Browse files Browse the repository at this point in the history
We only import white-listed globals from the query string for security
and sanity reasons. To keep the BC break as small as possible, we add
the globals used by the core to the white-list, and stick with the
current way to trigger the plugin administration. Plugins already using
`XH_wantsPluginAdministration()` are supposed to work as before. Other
extensions will have to be adapted, especially if they're making use of
this global import "feature" elsewhere (i.e. not only for the plugin
administration).

Particularly note that the TinyMCE plugin's administration is broken by
this commit, but this could easily be fixed, and TinyMCE is most likely
going to replaced anyway.
  • Loading branch information
cmb69 committed Feb 22, 2017
1 parent 77352ca commit 6d70c27
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 14 deletions.
2 changes: 1 addition & 1 deletion cmsimple/adminfuncs.php
Original file line number Diff line number Diff line change
Expand Up @@ -1105,7 +1105,7 @@ function XH_adminJSLocalization()
*/
function XH_wantsPluginAdministration($pluginName)
{
return isset($GLOBALS[$pluginName]) && $GLOBALS[$pluginName] == 'true';
return (bool) preg_match('/(?:^|&)' . preg_quote($pluginName, '/') . '(?!=)/', sv('QUERY_STRING'));
}

?>
10 changes: 5 additions & 5 deletions cmsimple/classes/Controller.php
Original file line number Diff line number Diff line change
Expand Up @@ -404,17 +404,17 @@ public function setBackendF()
$f = 'userfiles';
} elseif ($file) {
$f = 'file';
} elseif (isset($phpinfo)) {
} elseif ($phpinfo) {
$f = 'phpinfo';
} elseif (isset($sysinfo)) {
} elseif ($sysinfo) {
$f = 'sysinfo';
} elseif (isset($xh_pagedata)) {
} elseif ($xh_pagedata) {
$f = 'xh_pagedata';
} elseif (isset($xh_backups)) {
} elseif ($xh_backups) {
$f = 'xh_backups';
} elseif ($settings) {
$f = 'settings';
} elseif (isset($xh_do_validate)) {
} elseif ($xh_do_validate) {
$f = 'do_validate';
} elseif ($validate) {
$f = 'validate';
Expand Down
7 changes: 4 additions & 3 deletions cmsimple/cms.php
Original file line number Diff line number Diff line change
Expand Up @@ -749,8 +749,9 @@

$temp = array(
'action', 'download', 'downloads', 'edit', 'file', 'function', 'images',
'login', 'logout', 'keycut', 'mailform', 'media', 'normal', 'print', 'search',
'selected', 'settings', 'sitemap', 'text', 'userfiles', 'validate', 'xhpages'
'login', 'logout', 'keycut', 'mailform', 'media', 'normal', 'phpinfo', 'print', 'search',
'selected', 'settings', 'sitemap', 'sysinfo', 'text', 'userfiles', 'validate', 'xhpages',
'xh_backups', 'xh_do_validate', 'xh_pagedata'
);
foreach ($temp as $i) {
initvar($i);
Expand Down Expand Up @@ -803,7 +804,7 @@
$su = $selected;
}
foreach ($rq as $i) {
if (!strpos($i, '=')) {
if (!strpos($i, '=') && in_array($i, $temp)) {
$GLOBALS[$i] = 'true';
}
}
Expand Down
7 changes: 2 additions & 5 deletions tests/unit/AdminfuncsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,15 +49,12 @@ public function testSaveContentsRequiresEditMode()
* Test XH_wantsPluginAdministration().
*
* @return void
*
* @global string Whether the pagemanager administration is requested.
*/
public function testWantsPluginAdministration()
{
global $pagemanager;

$pagemanager = 'true';
$_SERVER['QUERY_STRING'] = '&pagemanager&normal';
$this->assertTrue(XH_wantsPluginAdministration('pagemanager'));
unset($_SERVER['QUERY_STRING']);
}

/**
Expand Down

0 comments on commit 6d70c27

Please sign in to comment.