Understanding the register_globals problem

djmonkey1 · February 22, 2006

The register_globals problem is, I believe, going to be a constant headache until new standards are adopted and maintained.

Currently patches are available for core osC files, however many contributions are written and tested on servers with globals turned "on" and so users who face the reality of having them turned "off" often find that while they can get their stock store to function, contributions fail to work properly.

It behooves developers of contributions to write their projects in such a way that they will function for both sets of users, the "off" and "on" crowds.

The problem I face, and I imagine others as well, is a lack of understanding of what exactly the problem is and how to correct for it.

In my limited understanding, the problem lies with the use of generic variables such as $name and $email. In order for these variables to function properly with register_globals turned "off", they would need to be rewritten in the form of

{_POST['name']}

or

{_GET['name']}

and if the version of PHP is prior to 4.1 it would have be

{HTTP_POST_VARS['name']}

or

{HTTP_GET_VARS['name']}

The question for me then, is the {HTTP_POST_VARS['name']} style format the way to go? Or will support for this be dropped shortly? Is this format supported, for instance, in PHP 5.x?

Furthermore, when and how should we convert variables into the new format(s)?

Take for instance, this section of code from the top of edit_orders.php from the Order Editor contribution (one of the ones that I have heard fails when globals are turned off):

// New "Status History" table has different format.
 $OldNewStatusValues = (tep_field_exists(TABLE_ORDERS_STATUS_HISTORY, "old_value") && tep_field_exists(TABLE_ORDERS_STATUS_HISTORY, "new_value"));
 $CommentsWithStatus = tep_field_exists(TABLE_ORDERS_STATUS_HISTORY, "comments");
 $SeparateBillingFields = tep_field_exists(TABLE_ORDERS, "billing_name");

 $orders_statuses = array();
 $orders_status_array = array();
 $orders_status_query = tep_db_query("select orders_status_id, orders_status_name from " . TABLE_ORDERS_STATUS . " where language_id = '" . (int)$languages_id . "'");
 while ($orders_status = tep_db_fetch_array($orders_status_query)) {
$orders_statuses[] = array('id' => $orders_status['orders_status_id'],
						   'text' => $orders_status['orders_status_name']);
$orders_status_array[$orders_status['orders_status_id']] = $orders_status['orders_status_name'];
 }

 $action = (isset($HTTP_GET_VARS['action']) ? $HTTP_GET_VARS['action'] : 'edit');

Does every instance of $variable (for instance, $OldNewStatusValues) need to be replaced?

I look forward to a discussion of this.

Cheers

Stew

♥FWR Media · February 22, 2006

The register_globals problem is, I believe, going to be a constant headache until new standards are adopted and maintained.

Currently patches are available for core osC files, however many contributions are written and tested on servers with globals turned "on" and so users who face the reality of having them turned "off" often find that while they can get their stock store to function, contributions fail to work properly.

It behooves developers of contributions to write their projects in such a way that they will function for both sets of users, the "off" and "on" crowds.

The problem I face, and I imagine others as well, is a lack of understanding of what exactly the problem is and how to correct for it.

In my limited understanding, the problem lies with the use of generic variables such as $name and $email. In order for these variables to function properly with register_globals turned "off", they would need to be rewritten in the form of

{_POST['name']}

or

{_GET['name']}

and if the version of PHP is prior to 4.1 it would have be

{HTTP_POST_VARS['name']}

or

{HTTP_GET_VARS['name']}

The question for me then, is the {HTTP_POST_VARS['name']} style format the way to go? Or will support for this be dropped shortly? Is this format supported, for instance, in PHP 5.x?

Furthermore, when and how should we convert variables into the new format(s)?

Take for instance, this section of code from the top of edit_orders.php from the Order Editor contribution (one of the ones that I have heard fails when globals are turned off):
// New "Status History" table has different format.
 $OldNewStatusValues = (tep_field_exists(TABLE_ORDERS_STATUS_HISTORY, "old_value") && tep_field_exists(TABLE_ORDERS_STATUS_HISTORY, "new_value"));
 $CommentsWithStatus = tep_field_exists(TABLE_ORDERS_STATUS_HISTORY, "comments");
 $SeparateBillingFields = tep_field_exists(TABLE_ORDERS, "billing_name");

 $orders_statuses = array();
 $orders_status_array = array();
 $orders_status_query = tep_db_query("select orders_status_id, orders_status_name from " . TABLE_ORDERS_STATUS . " where language_id = '" . (int)$languages_id . "'");
 while ($orders_status = tep_db_fetch_array($orders_status_query)) {
$orders_statuses[] = array('id' => $orders_status['orders_status_id'],
						   'text' => $orders_status['orders_status_name']);
$orders_status_array[$orders_status['orders_status_id']] = $orders_status['orders_status_name'];
 }

 $action = (isset($HTTP_GET_VARS['action']) ? $HTTP_GET_VARS['action'] : 'edit');
Does every instance of $variable (for instance, $OldNewStatusValues) need to be replaced?

I look forward to a discussion of this.

Cheers

Stew

I'm not the best person to answer this as I'm only a "part time" coder, but I'm working atm on a modified version of oscommerce that runs as a function of another script. Part of this involves changeing the variables and working globals off.

My understanding is that ..

$_GET directly replaces $HTTP_GET_VARS retrieving values from the query string

$_POST directly replaces $HTTP_POST_VARS retrieving values from posted forms etc etc.

$_REQUEST retrieves either get or post values

$_GET, $_POST do not need to be declared to be global.

It is also worth knowing that atm both $_GET and $HTTP_GET_VARS (and the others) are seen by php as two entirely seperate variables.

An excellent source of reference for predefined variables

$_GET['variablename'] replaces $HTTP_GET_VARS['variablename'] which is now deprecated.

Hope it helps.

kgt · February 22, 2006

Does every instance of $variable (for instance, $OldNewStatusValues) need to be replaced?

This sentence leads me to believe that you're unclear as to what register_globals is. The PHP manual describes in detail what happens:

http://us2.php.net/register_globals

Basically, only POST or GET data (form submissions), sessions (information saved by the server), cookies (information saved by the client browser), and webserver information (details about the script and the server) are affected by the register_globals directive. (There are a few others, not as often used: http://us2.php.net/manual/en/language.vari...predefined.php)

When you submit a form, all of the text fields have a name. This name is how PHP knows which field is which. The register_globals directive will turn the names of the submitted text fields into variables. It automatically does this:

$first_name = $_POST['first_name'];

This happens for cookie and session data as well. It does not change the way normal variables work. The following script will work on any machine properly running PHP (with register_globals on or off):

<?php

$text = "Hi there.";

echo $text;

?>

Only if I have a form with a text field named "text" that submits to this code will I have a problem depending on the status of the register_globals directive.

Using superglobal arrays always works, however. When coding, it's generally a good idea to use superglobal arrays, since they work on all environments.

kgt · February 22, 2006

$_GET directly replaces $HTTP_GET_VARS retrieving values from the query string
$_POST directly replaces $HTTP_POST_VARS retrieving values from posted forms etc etc.

$_REQUEST retrieves either get or post values

$_REQUEST is a superglobal that has the contents of $_POST, $_GET, and $_COOKIE. It is also a separate variable, so changing the $_POST array will have no effect on the $_REQUEST array.

Important to remember.

djmonkey1 · February 23, 2006

This sentence leads me to believe that you're unclear as to what register_globals is.

Yes, that's what I was getting at. :)

Who's up for some examples of variables that would need to be modified for globals being off, as well as some examples of variables that are ok?

insomniac2 · February 23, 2006

Very interesting topic. This has been bugging me too!. I am sooo confused after hours and hours of reading posts and researching web sites on this topic. Not to mentions the time spent mucking around in files changing values etc.

I have tried the osc with register globals off several times and have got it working well except for the odd glitch here and there which pops up as you explain.

For instance everything seems to work well except for admin orders.php ... which when I select an order and try to view ... it just refreshes the page with no errors but doesn't go to the order. I did manage to get it to work after changing a lot of code.

In catalog account_history.php when you try to view the details of a previous order ... same thing ... page refreshes no errors ... but doesn't go to the order.

Also if you use the loginbox contribution ... which puts a loginbox in the left or right column ... mine stops allowing me to log in. I can only login from the main login.php

I guess it boils down to ... no way around it. If we want register globals off we are all goin to spend a lot of time modifying existing code. And what works for one person ... doesn't necessarily work for the rest of us.

So now to get back on topic ... LOL ... what I myself would like to know is ... just to get somewhat of a start ...

Can ... Or is it wise to go through all of the osc files and change every instance of $HTTP_GET_VARS and $HTTP_POST_VARS etc. ........ TO $_POST ... $_GET etc. ....... without totally screwing up the shop?

I have tried it and it seemed to screw up a lot of things. Even putting (int) in front of variables like this (int)$oID ... or (int)$customer_id etc ... even though from what I understand are pure integers ... has screwed things up.

Anyway I am probably confusing people so I`ll shut up. But if I could figure it out .. I would gladly go through every file of the base osc install and change all of the variables to $_GET .. $_POST etc. ..... if it meant saving people the hassle of having to go through Hell ... LOL I know what it's like to be frustrated and pulling my hair out because the whole thing seems unstable. Just when you think you have everything running smooth ... more annoying glitches pop up ... or your database gets Trashed from screwy code.

Whewwww ... OK Tell me to shut up .... LOL

Anyway would appreciate any good input from say maybe ... someone who does know what their talking about LOL.

djmonkey1 · February 23, 2006

Can ... Or is it wise to go through all of the osc files and change every instance of $HTTP_GET_VARS and $HTTP_POST_VARS etc. ........ TO $_POST ... $_GET etc. ....... without totally screwing up the shop?

That one thing I believe I can comment on- the answer should be yes, you can do it, as long as you're using PHP 4.2 (or 4.1 depending on who you ask) or later.

However, in my (obviously limited) understanding, if you're already using $HTTP_GET_VARS['variable'] you shouldn't have a problem with register_globals (unless maybe there's a version of PHP so bleeding edge it no longer recognizes the HTTP version of the arrays).

Harald discusses this issue in the STANDARD file released with osC:

Variable Scope*
--------------

All variables must be accessed and set within their scope as:

$HTTP_GET_VARS['variable']

$HTTP_POST_VARS['variable']

$HTTP_COOKIE_VARS['variable']

$variable (either local, or session)

* This needs to be updated when the codebase has been made compatible with

the register_global parameter. Session variables are then accessed and set

within its scope as:

$HTTP_SESSION_VARS['variable']

When PHP3 support is dropped, the following scope will be used:

$_GET['variable']

$_POST['variable']

$_COOKIE['variable']

$_SESSION['variable']

PHP 4.0.x does not support the above scope which was introduced in PHP 4.1.x.

The following can be used which is not compatible with PHP 3.x:

$_GET =& $HTTP_GET_VARS;

$_POST =& $HTTP_POST_VARS;

$_COOKIE =& $HTTP_COOKIE_VARS;

$_SESSION =& $HTTP_SESSION_VARS;

My main problem is identifying the session variables that will need to be updated. kgt has provided this useful information:

Basically, only POST or GET data (form submissions), sessions (information saved by the server), cookies (information saved by the client browser), and webserver information (details about the script and the server) are affected by the register_globals directive. (There are a few others, not as often used: http://us2.php.net/manual/en/language.vari...predefined.php)

but I still feel dense on the issue. An example, an example, my kingdom for an example!

Also, there's the security issue, the underlying reason behind changing register_globals to "off" by default. Shouldn't the patches be applied even if you have globals turned on, for security? And shouldn't you likewise write code as if globals are turned off, once again to ensure a secure site?

djmonkey1 · February 23, 2006

I think I'm starting to get it.

The old way, with register_globals turned on, if you have a piece of code such as:

$lc_text = '<form name="buy_now_' . $listing[$x]['products_id'] . '" method="post" action="' . tep_href_link(basename($PHP_SELF), tep_get_all_get_params(array('action')) . 'action=buy_now_form', 'NONSSL') . '">';

then the script will automatically generate the variable $buy_now_' . $listing[$x]['products_id'] . ' and this variable will be available for use when the form is submitted.

But with register_globals turned off, the variable is not created automatically, and so any attempt to use it will result in an undefined variable error.

So, if this assessment is correct, what is the correct way to write the form tag to correct for this? Or is it a matter of changing the way the form is submitted?

♥FWR Media · February 23, 2006

I think I'm starting to get it.

The old way, with register_globals turned on, if you have a piece of code such as:
$lc_text = '<form name="buy_now_' . $listing[$x]['products_id'] . '" method="post" action="' . tep_href_link(basename($PHP_SELF), tep_get_all_get_params(array('action')) . 'action=buy_now_form', 'NONSSL') . '">';
then the script will automatically generate the variable $buy_now_' . $listing[$x]['products_id'] . ' and this variable will be available for use when the form is submitted.

But with register_globals turned off, the variable is not created automatically, and so any attempt to use it will result in an undefined variable error.

So, if this assessment is correct, what is the correct way to write the form tag to correct for this? Or is it a matter of changing the way the form is submitted?

Let's say a form is ..

</form>

action="gohere.php" takes you to gohere.php after the form is submitted

<input type="hidden" name="variable_check" value="a_flock_of_seagulls" /> Creates a variable which can be accessed by $_POST['variable_check']

Therefore anywhere .. or globally...

if (isset($_POST['variable_check'])) //Checks to see if $_POST['variable_check'] exists
echo $_POST['variable_check'];

would produce

a_flock_of_seagulls

I'll leave the globals on description for someone else, got to go.

djmonkey1 · February 23, 2006

Ok, so we'll look at some code from the Order Editor contribution.

This contribution features block on block of form data, like so:

<tr>
<td class="main"><b><?php echo ENTRY_CUSTOMER_COMPANY; ?>: </b></td>
<td><span class="main"><input name="update_customer_company" size="25" value="<?php echo tep_html_quotes($order->customer['company']); ?>"></span></td>
	<td> </td>
<td><span class="main"><input name="update_delivery_company" size="25" value="<?php echo tep_html_quotes($order->delivery['company']); ?>"></span></td>
<td> </td>
<td><span class="main"><input name="update_billing_company" size="25" value="<?php echo tep_html_quotes($order->billing['company']); ?>"></span></td>
 </tr>

My understanding of how this works is that it's all submitted by this section of code:

// 1.1 UPDATE ORDER INFO #####

	$UpdateOrders = "update " . TABLE_ORDERS . " set 
		customers_name = '" . tep_db_input(stripslashes($update_customer_name)) . "',
		customers_company = '" . tep_db_input(stripslashes($_POST['update_customer_company'])) . "',
		customers_street_address = '" . tep_db_input(stripslashes($update_customer_street_address)) . "',
		customers_suburb = '" . tep_db_input(stripslashes($update_customer_suburb)) . "',
		customers_city = '" . tep_db_input(stripslashes($update_customer_city)) . "',
		customers_state = '" . tep_db_input(stripslashes($update_customer_state)) . "',
		customers_postcode = '" . tep_db_input($update_customer_postcode) . "',
		customers_country = '" . tep_db_input(stripslashes($update_customer_country)) . "',
		customers_telephone = '" . tep_db_input($update_customer_telephone) . "',
		customers_email_address = '" . tep_db_input($update_customer_email_address) . "',";

	if($SeparateBillingFields) { 
	// Original: all database fields point to $update_billing_xxx, now they are updated with the same values as the customer fields
	$UpdateOrders .= "billing_name = '" . tep_db_input(stripslashes($update_billing_name)) . "',
		billing_company = '" . tep_db_input(stripslashes($update_billing_company)) . "',
		billing_street_address = '" . tep_db_input(stripslashes($update_billing_street_address)) . "',
		billing_suburb = '" . tep_db_input(stripslashes($update_billing_suburb)) . "',
		billing_city = '" . tep_db_input(stripslashes($update_billing_city)) . "',
		billing_state = '" . tep_db_input(stripslashes($update_billing_state)) . "',
		billing_postcode = '" . tep_db_input($update_billing_postcode) . "',
		billing_country = '" . tep_db_input(stripslashes($update_billing_country)) . "',";
	}

	$UpdateOrders .= "delivery_name = '" . tep_db_input(stripslashes($update_delivery_name)) . "',
		delivery_company = '" . tep_db_input(stripslashes($update_delivery_company)) . "',
		delivery_street_address = '" . tep_db_input(stripslashes($update_delivery_street_address)) . "',
		delivery_suburb = '" . tep_db_input(stripslashes($update_delivery_suburb)) . "',
		delivery_city = '" . tep_db_input(stripslashes($update_delivery_city)) . "',
		delivery_state = '" . tep_db_input(stripslashes($update_delivery_state)) . "',
		delivery_postcode = '" . tep_db_input($update_delivery_postcode) . "',
		delivery_country = '" . tep_db_input(stripslashes($update_delivery_country)) . "',
		payment_method = '" . tep_db_input($update_info_payment_method) . "',
		cc_type = '" . tep_db_input($update_info_cc_type) . "',
		cc_owner = '" . tep_db_input($update_info_cc_owner) . "',";

Note that I have changed one variable, $update_customer_company, to $_POST['update_customer_company'], and have found that it works, on my server, both ways.

Am I correct, then, that this is a huge block of code that will fail when register_globals is turned off, and that it should all be modifed in order to comply with the new standard of register_globals being turned off by default?

♥FWR Media · February 23, 2006

Ok, so we'll look at some code from the Order Editor contribution.

This contribution features block on block of form data, like so:

<tr>
<td class="main"><b><?php echo ENTRY_CUSTOMER_COMPANY; ?>: </b></td>
<td><span class="main"><input name="update_customer_company" size="25" value="<?php echo tep_html_quotes($order->customer['company']); ?>"></span></td>
	<td> </td>
<td><span class="main"><input name="update_delivery_company" size="25" value="<?php echo tep_html_quotes($order->delivery['company']); ?>"></span></td>
<td> </td>
<td><span class="main"><input name="update_billing_company" size="25" value="<?php echo tep_html_quotes($order->billing['company']); ?>"></span></td>
 </tr>

My understanding of how this works is that it's all submitted by this section of code:

// 1.1 UPDATE ORDER INFO #####

	$UpdateOrders = "update " . TABLE_ORDERS . " set 
		customers_name = '" . tep_db_input(stripslashes($update_customer_name)) . "',
		customers_company = '" . tep_db_input(stripslashes($_POST['update_customer_company'])) . "',
		customers_street_address = '" . tep_db_input(stripslashes($update_customer_street_address)) . "',
		customers_suburb = '" . tep_db_input(stripslashes($update_customer_suburb)) . "',
		customers_city = '" . tep_db_input(stripslashes($update_customer_city)) . "',
		customers_state = '" . tep_db_input(stripslashes($update_customer_state)) . "',
		customers_postcode = '" . tep_db_input($update_customer_postcode) . "',
		customers_country = '" . tep_db_input(stripslashes($update_customer_country)) . "',
		customers_telephone = '" . tep_db_input($update_customer_telephone) . "',
		customers_email_address = '" . tep_db_input($update_customer_email_address) . "',";

	if($SeparateBillingFields) { 
	// Original: all database fields point to $update_billing_xxx, now they are updated with the same values as the customer fields
	$UpdateOrders .= "billing_name = '" . tep_db_input(stripslashes($update_billing_name)) . "',
		billing_company = '" . tep_db_input(stripslashes($update_billing_company)) . "',
		billing_street_address = '" . tep_db_input(stripslashes($update_billing_street_address)) . "',
		billing_suburb = '" . tep_db_input(stripslashes($update_billing_suburb)) . "',
		billing_city = '" . tep_db_input(stripslashes($update_billing_city)) . "',
		billing_state = '" . tep_db_input(stripslashes($update_billing_state)) . "',
		billing_postcode = '" . tep_db_input($update_billing_postcode) . "',
		billing_country = '" . tep_db_input(stripslashes($update_billing_country)) . "',";
	}

	$UpdateOrders .= "delivery_name = '" . tep_db_input(stripslashes($update_delivery_name)) . "',
		delivery_company = '" . tep_db_input(stripslashes($update_delivery_company)) . "',
		delivery_street_address = '" . tep_db_input(stripslashes($update_delivery_street_address)) . "',
		delivery_suburb = '" . tep_db_input(stripslashes($update_delivery_suburb)) . "',
		delivery_city = '" . tep_db_input(stripslashes($update_delivery_city)) . "',
		delivery_state = '" . tep_db_input(stripslashes($update_delivery_state)) . "',
		delivery_postcode = '" . tep_db_input($update_delivery_postcode) . "',
		delivery_country = '" . tep_db_input(stripslashes($update_delivery_country)) . "',
		payment_method = '" . tep_db_input($update_info_payment_method) . "',
		cc_type = '" . tep_db_input($update_info_cc_type) . "',
		cc_owner = '" . tep_db_input($update_info_cc_owner) . "',";

Note that I have changed one variable, $update_customer_company, to $_POST['update_customer_company'], and have found that it works, on my server, both ways.

Am I correct, then, that this is a huge block of code that will fail when register_globals is turned off, and that it should all be modifed in order to comply with the new standard of register_globals being turned off by default?

Haven't time to answer that but here is a quick description of the difference between globals on/off.

Take a URL

http://www.example.com/test.php

With globals on if I add to the url admin=1

http://www.example.com/test.php?admin=1

This automatically creates a global variable $admin with value set to 1 or "true" this will declare a global variable with no other code required.

So theoretically I could log in as admin if the script checked if ($admin == true);

With globals off all I have created is a variable that can be accessed as $_GET['']

djmonkey1 · February 23, 2006

Haven't time to answer that but here is a quick description of the difference between globals on/off.

Take a URL

http://www.example.com/test.php

With globals on if I add to the url admin=1

http://www.example.com/test.php?admin=1

This automatically creates a global variable $admin with value set to 1 or "true" this will declare a global variable with no other code required.

So theoretically I could log in as admin if the script checked if ($admin == true);

With globals off all I have created is a variable that can be accessed as $_GET['']

That's why globals should be turned off, because of the possibility of things like this.

kgt · February 23, 2006

That's why globals should be turned off, because of the possibility of things like this.

Yes, but what needs to be understood about the register_globals directive is that it's not inherently insecure. Also, it's not as simple as choosing any variable and setting it to a desired value. If the programmer violates a basic standard (Initialize Your Variables), then this may be taken advantage of.

Consider the following URL and assume the ability to bypass security by adding admin=1 is unintended:

http://www.example.com/test.php?admin=1

The script would look something like this:

<?php
if( $admin ) {
echo "Admitted.";
//grant access to super-secret stuff
} else {
"Denied.";
}
?>

Now consider that script with the "proper" (intended) URL:

http://www.example.com/test.php

Our script would always output Denied with this URL because the value $admin is not set. Now what kind of buggy script is this? It would never make sense to base an IF condition on a variable that may or may not be set, especially one that is used to determine access privileges.

In most scripts, you'd always initialize a variable by setting it to some value. In doing so, you overwrite the initial value set in the URL. So the following script:

<?php

$admin = check_permissions(); //assume this function checks permissions and all that good stuff and returns true or false

if( $admin ) {

echo "Admitted.";

//grant access to super-secret stuff

} else {

"Denied.";

}

?>

Is not vulnerable to an attack by using the URL http://www.example.com/test.php?admin=1. It will always overwrite the value of $admin with the script's own value. You wouldn't be able to bypass security unless the code is buggy to begin with.

Having register_globals set to on does not necessarily mean any Tom, Dick, or Harry can come along and hack into your admin tool. It still requires that the code have a bug.

All that being said, there's no reason to even allow for the possibility of this happening, so the best course of action is to simply turn register_globals off.

AlanR · February 23, 2006

Having register_globals set to on does not necessarily mean any Tom, Dick, or Harry can come along and hack into your admin tool. It still requires that the code have a bug.

All that being said, there's no reason to even allow for the possibility of this happening, so the best course of action is to simply turn register_globals off.

You could also add that the base osC scripts are pretty secure in this regard. Especially after five years of debugging and fighting off hacks. If it were vulnerable to such simple things we'd have many more posts here reporting hacks.

djmonkey1 · February 24, 2006

Ok, so we'll look at some code from the Order Editor contribution.

This contribution features block on block of form data, like so:

<tr>
<td class="main"><b><?php echo ENTRY_CUSTOMER_COMPANY; ?>: </b></td>
<td><span class="main"><input name="update_customer_company" size="25" value="<?php echo tep_html_quotes($order->customer['company']); ?>"></span></td>
	<td> </td>
<td><span class="main"><input name="update_delivery_company" size="25" value="<?php echo tep_html_quotes($order->delivery['company']); ?>"></span></td>
<td> </td>
<td><span class="main"><input name="update_billing_company" size="25" value="<?php echo tep_html_quotes($order->billing['company']); ?>"></span></td>
 </tr>

My understanding of how this works is that it's all submitted by this section of code:

// 1.1 UPDATE ORDER INFO #####

	$UpdateOrders = "update " . TABLE_ORDERS . " set 
		customers_name = '" . tep_db_input(stripslashes($update_customer_name)) . "',
		customers_company = '" . tep_db_input(stripslashes($_POST['update_customer_company'])) . "',
		customers_street_address = '" . tep_db_input(stripslashes($update_customer_street_address)) . "',
		customers_suburb = '" . tep_db_input(stripslashes($update_customer_suburb)) . "',
		customers_city = '" . tep_db_input(stripslashes($update_customer_city)) . "',
		customers_state = '" . tep_db_input(stripslashes($update_customer_state)) . "',
		customers_postcode = '" . tep_db_input($update_customer_postcode) . "',
		customers_country = '" . tep_db_input(stripslashes($update_customer_country)) . "',
		customers_telephone = '" . tep_db_input($update_customer_telephone) . "',
		customers_email_address = '" . tep_db_input($update_customer_email_address) . "',";

	if($SeparateBillingFields) { 
	// Original: all database fields point to $update_billing_xxx, now they are updated with the same values as the customer fields
	$UpdateOrders .= "billing_name = '" . tep_db_input(stripslashes($update_billing_name)) . "',
		billing_company = '" . tep_db_input(stripslashes($update_billing_company)) . "',
		billing_street_address = '" . tep_db_input(stripslashes($update_billing_street_address)) . "',
		billing_suburb = '" . tep_db_input(stripslashes($update_billing_suburb)) . "',
		billing_city = '" . tep_db_input(stripslashes($update_billing_city)) . "',
		billing_state = '" . tep_db_input(stripslashes($update_billing_state)) . "',
		billing_postcode = '" . tep_db_input($update_billing_postcode) . "',
		billing_country = '" . tep_db_input(stripslashes($update_billing_country)) . "',";
	}

	$UpdateOrders .= "delivery_name = '" . tep_db_input(stripslashes($update_delivery_name)) . "',
		delivery_company = '" . tep_db_input(stripslashes($update_delivery_company)) . "',
		delivery_street_address = '" . tep_db_input(stripslashes($update_delivery_street_address)) . "',
		delivery_suburb = '" . tep_db_input(stripslashes($update_delivery_suburb)) . "',
		delivery_city = '" . tep_db_input(stripslashes($update_delivery_city)) . "',
		delivery_state = '" . tep_db_input(stripslashes($update_delivery_state)) . "',
		delivery_postcode = '" . tep_db_input($update_delivery_postcode) . "',
		delivery_country = '" . tep_db_input(stripslashes($update_delivery_country)) . "',
		payment_method = '" . tep_db_input($update_info_payment_method) . "',
		cc_type = '" . tep_db_input($update_info_cc_type) . "',
		cc_owner = '" . tep_db_input($update_info_cc_owner) . "',";

Note that I have changed one variable, $update_customer_company, to $_POST['update_customer_company'], and have found that it works, on my server, both ways.

Am I correct, then, that this is a huge block of code that will fail when register_globals is turned off, and that it should all be modifed in order to comply with the new standard of register_globals being turned off by default?

kgt · February 24, 2006

Am I correct, then, that this is a huge block of code that will fail when register_globals is turned off, and that it should all be modifed in order to comply with the new standard of register_globals being turned off by default?

Yes.

djmonkey1 · February 24, 2006

Yes.

Eureka! Thank you!

djmonkey1 · March 2, 2006

So how to I re-write foreach statements?

I'm working on Order Editor and I'm getting this when I try to update an order:

Warning:  Invalid argument supplied for foreach() in /admin/edit_orders.php on line 166

 Warning:  Invalid argument supplied for foreach() in /admin/edit_orders.php on line 236

 Warning:  Invalid argument supplied for foreach() in /admin/edit_orders.php on line 258

 Warning:  Invalid argument supplied for foreach() in /admin/edit_orders.php on line 269

The lines in question are:

foreach($update_totals as $total_details) {

foreach($update_totals as $total_index => $total_details)	{

foreach($update_totals as $total_details)	{

foreach($update_totals as $total_index => $total_details)

I've been digging around on this one but I don't get it.

djmonkey1 · March 2, 2006

foreach($_REQUEST['update_totals'] as $total_index => $total_details)	{

?

djmonkey1 · March 2, 2006

It's looking more and more like those errors were caused by something else wrong in the code I was using. In any case I've started over and I haven't seen them again, yet.

djmonkey1 · March 2, 2006

On second thought, the foreach errors may be caused by register_globals off. I've backtracked and now only see them when I'm not using

if(!empty($_GET)) extract($_GET);
if(!empty($_POST)) extract($_POST);

In other news, I've got this section of code:

if($CommentsWithStatus) {
		tep_db_query("insert into " . TABLE_ORDERS_STATUS_HISTORY . " 
			(orders_id, orders_status_id, date_added, customer_notified, comments) 
			values ('" . tep_db_input($_GET['oID']) . "', '" . tep_db_input($_POST['status']) . "', now(), " . tep_db_input($customer_notified) . ", '" . tep_db_input($_POST['comments'])  . "')");
		} else {
			if($OldNewStatusValues) {
			  tep_db_query("insert into " . TABLE_ORDERS_STATUS_HISTORY . " 
				(orders_id, new_value, old_value, date_added, customer_notified) 
				values ('" . tep_db_input($_GET['oID']) . "', '" . tep_db_input($_POST['status']) . "', '" . $order->info['orders_status'] . "', now(), " . tep_db_input($customer_notified) . ")");
			} else {
			  tep_db_query("insert into " . TABLE_ORDERS_STATUS_HISTORY . " 
				(orders_id, orders_status_id, date_added, customer_notified) 
				values ('" . tep_db_input($_GET['oID']) . "', '" . tep_db_input($_POST['status']) . "', now(), " . tep_db_input($customer_notified) . ")");
			}

and have found that if I change $customer_notified to $_POST['customer_notified'], it doesn't work and I get an "error in your MySQL syntax" message. It works fine as $customer_notified.

Any ideas on that one? I would think that variable would have to be set using $_POST.

kgt · March 2, 2006

if(!empty($_GET)) extract($_GET);
if(!empty($_POST)) extract($_POST);

Doing this is performing exactly what register_globals does.

and have found that if I change $customer_notified to $_POST['customer_notified'], it doesn't work and I get an "error in your MySQL syntax" message. It works fine as $customer_notified.

That's because 'customer_notify' is not the name of the POST variable. Look at the code for this checkbox and you'll find the real name.

djmonkey1 · March 2, 2006

Doing this is performing exactly what register_globals does.

According to what I'm reading it's a hack.

From the php manual:

Warning

Do not use extract() on untrusted data, like user-input ($_GET, ...). If you do, for example, if you want to run old code that relies on register_globals temporarily, make sure you use one of the non-overwriting extract_type values such as EXTR_SKIP and be aware that you should extract in the same order that's defined in variables_order within the php.ini.

But, that's what I've found people are using. It isn't making the code correct, it's making it functional.

That's because 'customer_notify' is not the name of the POST variable. Look at the code for this checkbox and you'll find the real name.

Good one- it's 'notify'. Thanks.

custodian · March 3, 2006

As other have discussed, I must ask.

Is this whole isse as simple as going through the files and simply replacing

$HTTP_GET_VARS to $_POST

$HTTP_POST_VARS to $_GET etc

...verbatim?

Or am I oversimplifying this?

Also.. will $_POST and $_GET work with globals ON? So that someone could slowly change the site over?

If the above verbatim replacements are true, couldn't this be as simple as using sed to replace these?

♥FWR Media · March 3, 2006

As other have discussed, I must ask.

Is this whole isse as simple as going through the files and simply replacing

$HTTP_GET_VARS to $_POST

$HTTP_POST_VARS to $_GET etc

...verbatim?

Or am I oversimplifying this?

Also.. will $_POST and $_GET work with globals ON? So that someone could slowly change the site over?

If the above verbatim replacements are true, couldn't this be as simple as using sed to replace these?

$_POST, $_GET and $_REQUEST work as long as it's a current(ish) version of PHP

It's nowhere near as simple as that. Whole sections of code need to be rewritten.

Archived

Understanding the register_globals problem

Recommended Posts

Share this post

Link to post

Share on other sites

Share this post

Link to post

Share on other sites

Share this post

Link to post

Share on other sites

Share this post

Link to post

Share on other sites

Share this post

Link to post

Share on other sites

Share this post

Link to post

Share on other sites

Share this post

Link to post

Share on other sites

Share this post

Link to post

Share on other sites

Share this post

Link to post

Share on other sites

Share this post

Link to post

Share on other sites

Share this post

Link to post

Share on other sites

Share this post

Link to post

Share on other sites

Share this post

Link to post

Share on other sites

Share this post

Link to post

Share on other sites

Share this post

Link to post

Share on other sites

Share this post

Link to post

Share on other sites

Share this post

Link to post

Share on other sites

Share this post

Link to post

Share on other sites

Share this post

Link to post

Share on other sites

Share this post

Link to post

Share on other sites

Share this post

Link to post

Share on other sites

Share this post

Link to post

Share on other sites

Share this post

Link to post

Share on other sites

Share this post

Link to post

Share on other sites

Share this post

Link to post

Share on other sites