Plugins and Inspeckt and $GLOBALS Plugins and Inspeckt and $GLOBALS
 

News:

cpg1.5.48 Security release - upgrade mandatory!
The Coppermine development team is releasing a security update for Coppermine in order to counter a recently discovered vulnerability. It is important that all users who run version cpg1.5.46 or older update to this latest version as soon as possible.
[more]

Main Menu

Plugins and Inspeckt and $GLOBALS

Started by gmc, December 19, 2013, 09:31:32 PM

Previous topic - Next topic

0 Members and 1 Guest are viewing this topic.

gmc

Writing/testing a rather extensive plugin (a topic for another thread soon I hope...) - but encountered an interesting issue dealing with register_globals...

In several cases I have the need to access a global (NOT input) variable from a function where the variable is either a choice of many - or constructed at execution time from other data... I access these variables using $GLOBALS['var_name'] which is documented as being equivalent to the 'global' statement in a function - but allows dynamically creating the variable name.
I see this technique used in several other places in CPG - including exif.php and install.php (though install doesn't execute the same code in init.inc.php - but invokes Inspekt itself - and doesn't clear $GLOBALS...)

This worked fine in my testing - until I tried a gallery with registered_globals set to ON...  (My normal galleries don't have this - but trying to test combinations/configurations other users might have...)
The $GLOBALS variable appears empty - and I don't see any means via Inspekt to access other that the normal 'EGPCS' variables...

I do see where $GLOBALS is emptied in 'init.inc.php' if register_globals is on after calling Inspekt - and commenting this out fixes the issue (I would have thought this would be invoked before any of my script variables were set in my admin script?? but it clearly impacts it.)

In any case - it seems the code in init.inc.php goes much further than the sanitization of input in the case of register_globals being set..
It would seem replacing:

// Set $strict to false to make the superglobals available
$strict = TRUE;

$superCage = Inspekt::makeSuperCage($strict);
// Remove any variables introduced by register_globals, if enabled
$keysToSkip = array('keysToSkip', 'register_globals_flag', 'superCage', 'cpg_time_start', 'key');

if ($register_globals_flag && is_array($GLOBALS)) {
    foreach ($GLOBALS as $key => $value) {
        if (!in_array($key, $keysToSkip) && isset($$key)) {
            unset($$key);
        }
    }
}


with:

// Set $strict to false to make the superglobals available
$strict = TRUE;

// Determine any variables introduced by register_globals, if enabled
if ($register_globals_flag) {
  $clean_requests = array();
  foreach ($_REQUEST as $key => $value) {
    $clean_requests[] = $key;
  }
}

$superCage = Inspekt::makeSuperCage($strict);

// Remove any variables introduced by register_globals, if enabled
if ($register_globals_flag) {
  foreach ($clean_requests as $key) {
     unset($$key);
  }
}

would achieve the desired result for GET, POST, COOKIE without the destruction of other script set values? (eliminates having to worry about 'keys to skip' as well.)
Could easily add SYSTEM and ENVIRONMENT variables to this logic if needed too...

Or is this expected/desired behavior?
Is there another way to access $GLOBALS not included in 'EGPCS' variables?

Thanks,
Greg

gallery.gmcdesign.com is my production gallery - but of course a testing plugin isn't there yet.
Thanks!
Greg
My Coppermine Gallery
Need a web hosting account? See my gallery for an offer for CPG Forum users.
Send me money

Αndré

Not sure what exactly you need to accomplish, but I assume you know this (Example #1)? Maybe we can avoid the use of $GLOBALS if it causes issues.

gmc

Trying not to rewrite/revise a lot of code...  :D
(actually been trying to find a way around it - but it will be very ugly...)
I started with a fixed list of variables I would need to access, and did use the 'normal' global statement in the functions. (Example 1)
I then found a need to expand and make the variable names accessed dependent on other variables passed - the number of variables needed and their names will vary based on input - it will raise maintenance issues if every time a new variable is added - a number of functions need to be updated as well.
This led to trying the $GLOBALS array as a means to access the data - and it worked very well.

The concern with register globals being on are the variables 'injected' into the script from the 'egpcs' SuperGlobal arrays - yet the code in Coppermine deletes all entries in $GLOBALS except for a few key variables chosen to save if register globals is on...
I'm still not sure why this affects variables I am setting in a plugin admin script (I would think init.inc.PHP was invoked BEFORE my Admin script, but it is clearly deleting variables I am setting.

The code I proposed addressed 'gpc' though as indicated it could easily accommodate 'es' as well (and should free memory used when done... Revised to address all variables affected by register globals being on:

// Set $strict to false to make the superglobals available
$strict = TRUE;

// Determine any variables introduced by register_globals, if enabled
if ($register_globals_flag) {
  $clean_requests = array();
  foreach ($_REQUEST as $key => $value) {
    $clean_requests[] = $key;
  }
  foreach ($_ENV as $key => $value) {
    $clean_requests[] = $key;
  }
  foreach ($_SERVER as $key => $value) {
    $clean_requests[] = $key;
  }
}

$superCage = Inspekt::makeSuperCage($strict);

// Remove any variables introduced by register_globals, if enabled
if ($register_globals_flag) {
  foreach ($clean_requests as $key) {
     unset($$key);
  }
  unset($clean_requests);
}


I would think the current code could cause issues in other areas - as I see use of the $GLOBALS variable elsewhere in CPG.
For the moment, I am just detecting that register globals is on and putting out an appropriate message... And still testing.

Thanks!
Greg
My Coppermine Gallery
Need a web hosting account? See my gallery for an offer for CPG Forum users.
Send me money

Αndré

The plugin engine is initialized after the cleanup of the superglobals. Can you please attach your plugin or an example plugin that demonstrates your behavior? Thanks.

gmc

Most certainly... Can easily demonstrate - just can't explain why it is doing what it is doing...
Clearly init.inc.php is processed before my admin script is called... but it is having some further effect.

Plugin gmc_test V0.6 attached (if you have a previous version from one of my earlier posts, please deinstall first.)

admin.php assigns 2 variables, then immediately calls a function to display the values:

$gmc_test_var1 = 'Variable 1 found';
$gmc_test_var2 = 'Variable 2 found';
gmc_test_display_vars();

Function gmc_test_display _vars displays the contents of the 2 variables - the first accessed via global statement, the second using the $GLOBALS array:

function gmc_test_display_vars() {
  global $gmc_test_var1;

  echo "Variable accessed via global statement in function: $gmc_test_var1<br>";
  echo 'Variable accessed via $GLOBALS array in function: '.$GLOBALS['gmc_test_var2'];
}


When installed and clicking Configure from plugin manager, the admin.php page will issue the following messages:
With register_globals OFF:
Variable accessed via global statement in function: Variable 1 found
Variable accessed via $GLOBALS array in function: Variable 2 found

With register_globals ON:
Variable accessed via global statement in function: Variable 1 found
Variable accessed via $GLOBALS array in function:

With register_globals ON and commenting out the code in init.inc.php that unsets keys in $GLOBALS (or using my proposed code):
Variable accessed via global statement in function: Variable 1 found
Variable accessed via $GLOBALS array in function: Variable 2 found

(I'll post in admin menu thread - this version of gmc_test plugin also shows the different admin button methods (selectable via config))

Happy New Year all!
Thanks!
Greg
My Coppermine Gallery
Need a web hosting account? See my gallery for an offer for CPG Forum users.
Send me money

Αndré

Unfortunately I just realized I cannot test with register_globals on, as that feature isn't available in my testbed (PHP 5.4). However, I recommend to use the global statement to access your variables. If you store a lot of stuff and don't want to globalize several variables, consider to use an array.

$my_globals['gmc_test_var1'] = 'Variable 1 found';
function gmc_test_display_vars() {
  global $my_globals;

  echo "Variable accessed via global statement in function: ".$my_globals['gmc_test_var1'];
}

gmc

Well... I've eliminated the 'easy to change' use of $GLOBALS... I'm down to a dozen references to $GLOBALS - but some are just making the code and maintenance too ugly... and they are already arrays in most cases.

I can set up access for you to some of my test galleries - set up with the various options and the gmc_test plugin - if that would help?
(would need to message you with id/password to access site and cpg).

While register_globals is deprecated - and gone in PHP 5.4 - I expect we will see older versions of PHP for some time.. Too much code still relies on that setting...

In the interim at least - I've added code to the plugin I'm testing to detect that the issue exists (register globals ON and $GLOBALS not available) - and will issue an appropriate message rather than letting things fail...
Thanks!
Greg
My Coppermine Gallery
Need a web hosting account? See my gallery for an offer for CPG Forum users.
Send me money

gmc

Αndré,

Resent test credentials via PM.
Thanks!
Greg
My Coppermine Gallery
Need a web hosting account? See my gallery for an offer for CPG Forum users.
Send me money