// includes/bootstrap.inc ~line 140

// I was getting a 'Notice: Undefined variable: GLOBALS in ' error here many times through the foreach loop
// I added the line to check that there was a variable to be unset
// I'm not sure why the foreach(...) would be looping if GLOBALS were not defined... hmm
// A better fix is to turn 'register_globals = Off' in php.ini, but I had it turned on for 4.6.6
// a local php.ini file wasn't working for me...
// this lets me continue with it on for continued testing with 4.6.6 on the same server

function drupal_unset_globals() {
if (ini_get('register_globals')) {
$allowed = array('_ENV' => 1, '_GET' => 1, '_POST' => 1, '_COOKIE' => 1, '_FILES' => 1, '_SERVER' => 1, '_REQUEST' => 1, 'access_check' => 1);
foreach ($GLOBALS as $key => $value) {
if (!isset($allowed[$key])) {
if (isset($GLOBALS[$key])) // avoid 'Notice: Undefined variable: GLOBALS in ...' error message
unset($GLOBALS[$key]);
}
}
}
}

CommentFileSizeAuthor
#7 globals_globals.patch651 byteschx
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

chx’s picture

chx CreditAttribution: chx commented
Status: Active » Closed (won't fix)

$GLOBALS

Contains a reference to every variable which is currently available within the global scope of the script. The keys of this array are the names of the global variables. $GLOBALS has existed since PHP 3.

atmanning’s picture

atmanning CreditAttribution: atmanning commented

I don't understand what "active >> won't fix" means

The call to check for isset($var) before unset($var) seems obvious, especially in this case since it generates so many ugly error messages on the initial install of the long anticipated version 4.7. Fortunately, many won't see it if they properly have register_globals = Off.

The blurb about what GLOBALS is supposed to be gives no insight into the error message or why the status == "won't fix". Please clarify.

markus_petrux’s picture

markus_petrux CreditAttribution: markus_petrux commented

isset here is redundant.

Note that the foreach look is walking the $GLOBALS array, so inside this loop $GLOBALS[$key] is always TRUE.

Have you found a problem when register_globals is On or Off? Maybe the check for ini_get('register_globals') needs to be fixed?

What version of PHP? Could you please check what does ini_get('register_globals') return in your server? Maybe creating a quick page with PHP format and entering something like this:

$result = ini_get('register_globals');
print "ini_get('register_globals') returns:";
var_dump($result);
print '';
atmanning’s picture

atmanning CreditAttribution: atmanning commented

I agree that the $globals[] should be defined inside the loop, so I was suprised that this fix was needed.
I'm running Apache2 with php 5.0.5

The results when register_globals = On: (this is when the error message is displayed without the isset() conditional)

ini_get('register_globals') returns:
string(1) "1"

with register_globals = Off
ini_get('register_globals') returns:
string(0) ""

further investigation reveals
In php 5.0.5 with reference_globals = On, the first element of $GLOBALS is a reference to itself (recursive)
so, here's the better fix

function drupal_unset_globals() {
  if (ini_get('register_globals')) {
    $allowed = array('_ENV' => 1, '_GET' => 1, '_POST' => 1, '_COOKIE' => 1, '_FILES' => 1, '_SERVER' => 1, '_REQUEST' => 1, 'access_check' => 1);
    foreach ($GLOBALS as $key => $value) {
      if (!isset($allowed[$key])) {
			if ($key != 'GLOBALS' ) 
				unset($GLOBALS[$key]);		
      }
    }
  }
}
markus_petrux’s picture

markus_petrux CreditAttribution: markus_petrux commented
Priority: Normal » Critical
Status: Closed (won't fix) » Active

Yup! I haven't seen this before, but there are, indeed, several references in google on $GLOBALS having a reference to itself. Including references to PHP bugs that mention this.

http://www.google.com/search?hl=en&q=php+%24GLOBALS+is+a+reference+to+it...

I tried in PHP 4.4.0 and no, not here. So it may depend on the PHP version.

Setting to open as critical as if it happens, this code snippet is unsetting $GLOBALS which may cause unexpected results (Drupal uses $GLOBALS here and there).

markus_petrux’s picture

markus_petrux CreditAttribution: markus_petrux commented
Title: simple fix for 'Undefined variable: GLOBALS' error message » The when $GLOBALS has a reference to itself story
chx’s picture

chx CreditAttribution: chx commented
Priority: Critical » Normal
Status: Active » Reviewed & tested by the community
FileSize
globals_globals.patch651 bytes

This is very easy to fix. Applies to 4.7.0 and HEAD both.

drumm’s picture

drumm
he/him
Location NY, US
CreditAttribution: drumm commented
Status: Reviewed & tested by the community » Fixed

Committed to HEAD.

killes@www.drop.org’s picture

killes@www.drop.org CreditAttribution: killes@www.drop.org commented

and to 4.7

atmanning’s picture

atmanning CreditAttribution: atmanning commented

We might want to make sure to do more testing with php 4 AND register_globals = on just in case there are other harder-to-find bugs in that scenario.

Is there a testin checklist somewhere?

pik’s picture

pik CreditAttribution: pik commented

I had this problem described above and I replaced this script:

function drupal_unset_globals() {
if (ini_get('register_globals')) {
$allowed = array('_ENV' => 1, '_GET' => 1, '_POST' => 1, '_COOKIE' => 1, '_FILES' => 1, '_SERVER' => 1, '_REQUEST' => 1, 'access_check' => 1);
foreach ($GLOBALS as $key => $value) {
if (!isset($allowed[$key])) {
unset($GLOBALS[$key]);
}
}
}
}

With this:

function drupal_unset_globals() {
if (ini_get('register_globals')) {
$allowed = array('_ENV' => 1, '_GET' => 1, '_POST' => 1, '_COOKIE' => 1, '_FILES' => 1, '_SERVER' => 1, '_REQUEST' => 1, 'access_check' => 1);
foreach ($GLOBALS as $key => $value) {
if (!isset($allowed[$key])) {
if ($key != 'GLOBALS' )
unset($GLOBALS[$key]);
}
}
}
}

In /includes/bootstrap.inc and it works for me.

Heine’s picture

Heine CreditAttribution: Heine commented

Patch in #7 is more elegant. In the mean time it has been tested by a lot of people (look at the forum) with success. Already commited.

Anonymous’s picture

(not verified) CreditAttribution: commented
Status: Fixed » Closed (fixed)
Chill35’s picture

Chill35 CreditAttribution: Chill35 commented

Heine says :

In the mean time it has been tested by a lot of people (look at the forum) with success. Already commited.

What behavior is this patch suppose to fix ?

I just installed 4.7.5 and I am experiencing major log in problems with it.

Heine’s picture

Heine CreditAttribution: Heine commented

@Chill35: please open a new issue with sufficient information. This patch went in long before 4.7.5 and fixes the unsetting of the entire $GLOBALS array when it contained a reference to itself. Unsetting parts of $GLOBALS is necessary when running with register_globals = on to prevent attackers from creating variables.