Get rid of arbitrary global variables

Discussions and requests related to new CMSimple features, plugins, templates etc. and how to develop.
Please don't ask for support at this forums!
Post Reply
cmb
Posts: 14225
Joined: Tue Jun 21, 2011 11:04 am
Location: Bingen, RLP, DE
Contact:

Get rid of arbitrary global variables

Post by cmb » Sat May 17, 2014 10:22 am

Hello Community,

IMO CMSimple uses way too many global variables, but especially irritating is the fact that all GET parameters without a value are promoted to globals. This happens in cmsimple/cms.php line 562 (XH 1.6.1):

Code: Select all

    foreach ($rq as $i) {
        if (!strpos($i, '=')) {
            $GLOBALS[$i] = 'true';
        }
    } 
To make it clear: that means that arbitrary global variables can be defined simply by passing the names as GET parameter![1] This faintly smells like register_globals=On, even though in an extremly weakend manner, and I don't consider this a real security issue, as an attacker can't inject an arbitrary value. Nonetheless I hope we can get rid of it as soon as possible.

The biggest problem (if not the only one) seem to be requests to the administration of a plugin. The customary idiom to handle the adminstration of a plugin (for instance, the "example" plugin) is to conditionally execute the required code (in the plugin's admin.php):

Code: Select all

if ($example) {...} 
or maybe somewhat cleaner:

Code: Select all

if (isset($example) && $example == 'true') {...} 
To avoid breaking existing plugins we can't change this behavior, and as it is not possible to detect the use of global variables, unfortunately, we can't even effectively deprecate it. The best idea I can come up with so far is to introduce a function that can be used for checking if a certain plugin's administration is requested, say XH_wantsPluginAdministration(). So the "example" plugin could use:

Code: Select all

if (XH_wantsPluginAdminstration('example')) {...} 
The implementation for now would be trivial:

Code: Select all

function XH_wantsPluginAdministration($pluginName)
{
    return $GLOBALS[$pluginName] == 'true';
} 
When we decide to actually remove these global variables, the implementation can be changed without affecting plugins using this function. Plugins, on the other hand, could already use the function if available, and fall back to checking the respective global:

Code: Select all

if (function_exists('XH_wantsPluginAdministration') 
    && XH_wantsPluginAdministration('example') 
    || $example
) {...}
I suggest to introduce the new function (maybe with another name) with CMSimple_XH 1.6.3, so plugin authors can already prepare their plugins then.

[1] I wonder why the value of these globals is set to 'true' (string) instead of true (boolean). Is that a relict of PHP 3, where there might have been no boolean variables available? :?

Christoph
Christoph M. Becker – Plugins for CMSimple_XH

cmb
Posts: 14225
Joined: Tue Jun 21, 2011 11:04 am
Location: Bingen, RLP, DE
Contact:

Re: Get rid of arbitrary global variables

Post by cmb » Wed Aug 13, 2014 6:35 pm

Done (r1344).

PS: I've already documented the function in the Plugin Tutorial.
Christoph M. Becker – Plugins for CMSimple_XH

Post Reply