Jump to content
  • Checkout
  • Login
  • Get in touch

osCommerce

The e-commerce.

Understanding the register_globals problem


djmonkey1

Recommended Posts

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?

thunderace is right- updating $HTTP_GET_VARS to $_GET and $HTTP_POST_VARS to $_POST is just that, an update (and won't work with versions of PHP prior to 4.1), but doesn't address the issue that having register_globals turned OFF brings up.

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 and $_GET will work with globals ON (as long as you're running PHP 4.1 or later), in fact code written for globals OFF will work perfectly with globals ON (which is one reason why I'm asking about all this- because if we all write all our code as if globals is OFF, then it will work far better for more people).

 

The reason why it's not as simple as replacing $HTTP_GET_VARS is addressed in the STANDARD file released with osC:

 

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']

 

The problem is with "$variable (either local, or session)", because these variables also rely on $_GET and $_POST, it's just with globals turned ON those arrays are set automatically even when they're not specifically referenced. See my example:

 

			customers_name = '" . tep_db_input(stripslashes($update_customer_name)) . "',
		customers_company = '" . tep_db_input(stripslashes($_POST['update_customer_company'])) . "',

 

Both lines rely on POST to send the information to the database, but only one specifically calls for it.

 

Both lines work exactly the same with globals turned ON.

 

But with globals OFF, the first line will return a blank value.

Do, or do not. There is no try.

 

Order Editor 5.0.6 "Ultra Violet" is now available!

For support or to post comments, suggestions, etc, please visit the Order Editor support thread.

Link to comment
Share on other sites

  • Replies 67
  • Created
  • Last Reply

Well, basically it all comes down to analyzing where the variables that are used in the script comes from and change it so that it retrieves from either &post,&get etc, and not just calls them directly.

 

In very short terms:

 

Globals on allows &GET,&POST,&SESSION etc variables to be accessed by simply &variable, while Globals off doesn't.

 

This is essentially more secure, as a buggy script is less lightly to be hacked this way, that's why a lot of hosts have it off. However, if you write your code correctly for Globals on, then it's just as secure.

 

But writing code for the masses would be best done Globals off, for compatility reasons.

 

It would be quite an undertaking to change it all, but it could be done. What I would do is simply install it on a server with globals off and then go through each file. Essentially all code written that works with globals off will automagically work with globals on.

 

It all comes down to one thing: Take control of your variables!

Insert clever remark here

Link to comment
Share on other sites

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)

If anyone is wondering about this:

foreach($_POST['update_totals'] as $total_details) {

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

Do, or do not. There is no try.

 

Order Editor 5.0.6 "Ultra Violet" is now available!

For support or to post comments, suggestions, etc, please visit the Order Editor support thread.

Link to comment
Share on other sites

Here's something for y'all to chew on. In my ongoing attempts to learn how to write for globals OFF by rewriting the Order Editor contribution, I've run into some particularly hairy code.

 

Basically, the only part of OE that I haven't updated for globals OFF are the steps for add a product. Step 1, in it's original incarnation, looks like this:

 

// Step 1: Choose Category
		print "<tr class=\"dataTableRow\"><form action='$PHP_SELF?oID=$oID&action=$action' method='POST'>\n";
		print "<td class='dataTableContent' align='right'><b>" . ADDPRODUCT_TEXT_STEP . " 1:</b></td>\n";
		print "<td class='dataTableContent' valign='top'>";
		echo ' ' . tep_draw_pull_down_menu('add_product_categories_id', tep_get_category_tree(), $current_category_id, 'onChange="this.form.submit();"');
		print "<input type='hidden' name='step' value='2'>";
		print "</td>\n";
		print "<td class='dataTableContent'>" . ADDPRODUCT_TEXT_STEP1 . "</td>\n";
		print "</form></tr>\n";
		print "<tr><td colspan='3'> </td></tr>\n";

 

Currently I'm working with this:

 

// Step 1: Choose Category
		echo '<tr class="dataTableRow"><form action=' . $_SERVER['PHP_SELF'] .'?oID=' . $_GET['oID'] . '&action=' . $_GET['action'] . ' method="POST">' . "\n";
		echo '<td class="dataTableContent" align="right"><b>' . ADDPRODUCT_TEXT_STEP . ' 1:</b></td>' .  "\n";
		echo '<td class="dataTableContent" valign="top">';
		echo ' ' . tep_draw_pull_down_menu('add_product_categories_id', tep_get_category_tree(), $_POST['current_category_id'], 'onChange="this.form.submit();"');
		echo '<input type="hidden" name="step" value="2">' . "\n";
		echo '</td>' . "\n";
		echo '<td class="dataTableContent">' . ADDPRODUCT_TEXT_STEP1 . '</td>' . "\n";
		echo '</form></tr>' . "\n";
		echo '<tr><td colspan="3"> </td></tr>' . "\n";

 

Now, obviously

echo '<input type="hidden" name="step" value="2">' . "\n";

will not work with globals OFF, but I can't for the life of me figure out how to fix it so it does. I tried setting up a variable I called $steptwo with a value of 2, then writing the input as

echo '<input type="hidden" name="step" value=' . $_POST['steptwo'] . '>' . "\n";

but that didn't work.

 

Additionally, I know that add_product_categories_id is not being set unless I use

link_post_variable('add_product_categories_id');

but I don't know why.

 

This is driving me nuts!

Do, or do not. There is no try.

 

Order Editor 5.0.6 "Ultra Violet" is now available!

For support or to post comments, suggestions, etc, please visit the Order Editor support thread.

Link to comment
Share on other sites

Now, obviously
echo '<input type="hidden" name="step" value="2">' . "\n";

will not work with globals OFF, but I can't for the life of me figure out how to fix it so it does. I tried setting up a variable I called $steptwo with a value of 2, then writing the input as

echo '<input type="hidden" name="step" value=' . $_POST['steptwo'] . '>' . "\n";

but that didn't work.

 

<input type="hidden" name="step" value="2">

 

is HTML and of course it will work with register_globals off. The NAME of this variable is "step". The VALUE of this variable is 2. So:

 

echo $_POST['step'];

 

prints out:

 

2

 

(If the form has been submitted, of course.)

Contributions

 

Discount Coupon Codes

Donations

Link to comment
Share on other sites

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

 

Correct, but it's pointless to do this if you're trying to rewrite your code to not use register_globals. The purpose of that 'hack' is to allow people hosted by servers with register_globals off to still run OSC as if register_globals were on.

Contributions

 

Discount Coupon Codes

Donations

Link to comment
Share on other sites

Correct, but it's pointless to do this if you're trying to rewrite your code to not use register_globals. The purpose of that 'hack' is to allow people hosted by servers with register_globals off to still run OSC as if register_globals were on.

 

exactly, register globals off does not make a site secure, it simply does not allow certain specific coding practices which may make a site less secure.

 

In other words, a site with globals on is just as secure as a site with globals off if they use the same coding standards.

Treasurer MFC

Link to comment
Share on other sites

To clarify, as I think some ppl don't really understand this:

 

Globals on/off is not an issue when you submit your form, but when you want to retrieve data from the form after it's submitted.

 

Personally I try to make all my code changes globals off friendly, but as I have a very osc friendly host I haven't really looked into the codebase.

Insert clever remark here

Link to comment
Share on other sites

Correct, but it's pointless to do this if you're trying to rewrite your code to not use register_globals. The purpose of that 'hack' is to allow people hosted by servers with register_globals off to still run OSC as if register_globals were on.

Well, uh, I still want my site to work in the interim. I don't want to use extract forever, just until I can get the code down.

Globals on/off is not an issue when you submit your form, but when you want to retrieve data from the form after it's submitted.

Of course.... my problem didn't lie in step 1, but in steps 2-5, where the form data is actually being used. This is like pulling teeth!

Do, or do not. There is no try.

 

Order Editor 5.0.6 "Ultra Violet" is now available!

For support or to post comments, suggestions, etc, please visit the Order Editor support thread.

Link to comment
Share on other sites

Also, if you're working on step 2, what doesn't work, you've only posted step 1, so it's hard to guess what's not wrking on step 2?

 

 

i'm currently converting my osCommerce to globals off and the way Im doing it is I'm collecting the $_GET/$_POST then moving it to an array e.g.

 

global $osc_var;

$osc_var['step'] = $_POST['step'];
$osc_var['another_variable'] = $_GET['another_variable'];

All my $osc_var variables are available to me throughout the script.

 

global $osc_var; needs to be early in the script e.g. top of application_top.php and you have to remember that if you're using $osc_var in a function then that function needs "global $osc_var;" at the top.

Link to comment
Share on other sites

i'm currently converting my osCommerce to globals off and the way Im doing it is I'm collecting the $_GET/$_POST then moving it to an array e.g.

 

global $osc_var;

$osc_var['step'] = $_POST['step'];
$osc_var['another_variable'] = $_GET['another_variable'];

All my $osc_var variables are available to me throughout the script.

 

global $osc_var; needs to be early in the script e.g. top of application_top.php and you have to remember that if you're using $osc_var in a function then that function needs "global $osc_var;" at the top.

 

why would you ever want to do something as futile as this?

Treasurer MFC

Link to comment
Share on other sites

why would you ever want to do something as futile as this?

 

 

Hmmm, well I don't see it as futile, I want my shop to run globals off without a "workaround" also I need it to run as a function so that files are called as e.g.

 

http://localhost/index.php?option=osc_shop&ref=product_info

Link to comment
Share on other sites

Hmmm, well I don't see it as futile, I want my shop to run globals off without a "workaround" also I need it to run as a function so that files are called as e.g.

 

http://localhost/index.php?option=osc_shop&ref=product_info

 

but this is nothing but a "workaround" and adds nothing but clutter to the normal usage of $_POST and $_GET.

Treasurer MFC

Link to comment
Share on other sites

I don't understand how copying data from an array into another array is advantageous. Especially since that second array must be declared global to be useful within functions...the superglobals arrays are already, well, global. I can understand the desire to make it run as a function, or a module of another codebase, but I'm not following how the array copy helps in that regard.

Contributions

 

Discount Coupon Codes

Donations

Link to comment
Share on other sites

Here is some info for the register globals and the osC contribution helps to clarify each case.

 

Working Example with register globals ON:

if($action == 'process' ) {
// Do Something
}

 

Minimum code needed to have the same code working when register globals are OFF and without the register global contribution installed (using $HTTP_GET_VARS):

if( isset($HTTP_GET_VARS['action'] ) ) {
 $action = $HTTP_GET_VARS['action'];
 if($action == 'process' ) {
 // Do Something
 }
}

 

The difference is the $action variable is clearly initialized when register globals are OFF. Now this contribution comes in to fill this gap. But it only takes into account the default osc code global variables (most of them). So if you install contribution-X that relies on some global variables and doesn't bother to initialize them, you have problems. So you either initialize them using the code above in which case you don't need the register global contribution or you use the wrappers included:

 

The equivalent of the complicated code above to initialize $action when the $action is passed using the $HTTP_GET_VARS global array:

// >>> BEGIN REGISTER_GLOBALS
link_get_variable('action');
// <<< END REGISTER_GLOBALS

 

Equivalent code when $action is passed using the $HTTP_POST_VARS global array

// >>> BEGIN REGISTER_GLOBALS
link_set_variable('action');
// <<< END REGISTER_GLOBALS

 

Normally the wrappers should be called after the application_top.php is included with the php script.

 

If you don't do that, the problems vary depending on the php scripts you have and obviously these problems can be severe, like having entire contributions/forms not working.

 

The difficulty is, you need to check every page more or less of your store (catalog & admin), to see, what code assumes globals are initialized automatically, and fix it.

Link to comment
Share on other sites

but this is nothing but a "workaround" and adds nothing but clutter to the normal usage of $_POST and $_GET.

 

 

I think you misunderstand (or perhaps I misexplain) I'm not converting $_GET/$_POST (well I am converting $HTTP_?_VARS to $_, but it's an aside not the goal) I am converting $GLOBALS to $osc_var[]

 

All i was saying is that with my system if I want to make use of a variable after a POST or GET I assign it to $osc_var[].

Link to comment
Share on other sites

I don't understand how copying data from an array into another array is advantageous. Especially since that second array must be declared global to be useful within functions...the superglobals arrays are already, well, global.

 

I understand what you're saying kgt, and maybe what I am doing is of no advantage to others, I will however release it as a contrib when ready.

 

basically I unset() $GLOBALS, and all $HTTP_ at the end of application_top, mainly to aid debugging, also error_reporting(E_ALL);

 

I wish to have a working osC where there are no $GLOBALS and all variables that I wish to be globally available either exist in $_SESSION or in $osc_var[].

 

Perhaps it's just what I'm used to.

Link to comment
Share on other sites

basically I unset() $GLOBALS, and all $HTTP_ at the end of application_top, mainly to aid debugging, also error_reporting(E_ALL);

 

I wish to have a working osC where there are no $GLOBALS and all variables that I wish to be globally available either exist in $_SESSION or in $osc_var[].

 

I still don't truly understand the point. So you're taking a global array, copying it to another array, declaring that one global, then deleting the first array. What does this accomplish? And how does it aid debugging? If anything, it adds another layer of complexity, not to mention unecessary overhead.

 

When you unset() $GLOBALS, are you unset()ing all 13 other superglobal arrays?

Contributions

 

Discount Coupon Codes

Donations

Link to comment
Share on other sites

...I want my shop to run globals off without a "workaround"...

Hello,

 

I don't wish to intrude on the converstation, but one thing that may be worth pointing out (and may help - dunno), I assume you are referring to the register globals contribution when you say "workaround"?

 

I know I refer to it as that in the patch, but one thing I would say is that (if I may be so modest), it is a GOOD "workaround" - ie - it doesn't just pretend to plug the register globals security hole - it really DOES plug the security hole.

 

So, personally, I can't really see the value of copying the $_GET / $_POST vars over to a custom (global) array either. I think you're just making work for yourself for little, if any, gain.

 

By the way, it's...

 

link_post_variable('action');

 

...and not...

 

link_set_variable('action');

 

regards

 

Rich.

Link to comment
Share on other sites

Hello,

 

I don't wish to intrude on the converstation, but one thing that may be worth pointing out (and may help - dunno), I assume you are referring to the register globals contribution when you say "workaround"?

 

I know I refer to it as that in the patch, but one thing I would say is that (if I may be so modest), it is a GOOD "workaround" - ie - it doesn't just pretend to plug the register globals security hole - it really DOES plug the security hole.

 

So, personally, I can't really see the value of copying the $_GET / $_POST vars over to a custom (global) array either. I think you're just making work for yourself for little, if any, gain.

 

By the way, it's...

 

link_post_variable('action');

 

...and not...

 

link_set_variable('action');

 

regards

 

Rich.

 

Hi Rich

 

A perfect opportunity for me to thank you for your register globals FIX.

 

I use it in a successful shop and it works flawlessly.

 

And I did mispresent myself earlier I DON'T :) duplicate $_GET / $_POST vars over to a custom (global) unless I wish to retain them for future use. What I do is use $osc_var[] for non local variables as opposed to $GLOBALS.

 

Does that make any sense?

Link to comment
Share on other sites

Hello,

 

I don't wish to intrude on the converstation, but one thing that may be worth pointing out (and may help - dunno), I assume you are referring to the register globals contribution when you say "workaround"?

 

I know I refer to it as that in the patch, but one thing I would say is that (if I may be so modest), it is a GOOD "workaround" - ie - it doesn't just pretend to plug the register globals security hole - it really DOES plug the security hole.

 

So, personally, I can't really see the value of copying the $_GET / $_POST vars over to a custom (global) array either. I think you're just making work for yourself for little, if any, gain.

 

By the way, it's...

 

link_post_variable('action');

 

...and not...

 

link_set_variable('action');

 

regards

 

Rich.

 

 

agree with most you said but this "it really DOES plug the security hole" you might want to substantiate a little.

Treasurer MFC

Link to comment
Share on other sites

yea it is link_post_variable my mistake. As of the security comments it simply enforces better coding and perhaps for those that haven't revised certain aspects of a code section could help on the one hand (by having some errors with the code) on the other hand you need to spend some time debugging the code.

 

Apart of these comments you also need to consider cases where a common parameter like "action" or "page" is incorrectly passed from one page to another. Simply because someone changed the filename on the tep_href_link but he forgot to change the parameters. In such case unfortunately register globals on/off make no difference and the side effects can be really bad.

Link to comment
Share on other sites

agree with most you said but this "it really DOES plug the security hole" you might want to substantiate a little.

Well, for example, one 'fix' that I have seen for this problem is a three-liner that just generates global variables for ALL $_GET and $_POST variables.

 

While it will 'work', This approach, is clearly no better than having register globals enabled in php.ini because any malicious variable injection will be passed through to the application with no complaint.

 

With the register globals patch, only the variables that are specifically required by the application are actually converted into globals (when I say 'globals', I mean globals that you can just access as $varname). In reality, there are actually very very few of these; the base osc code is surprisingly well written in this respect, in that it uses $HTTP_***_VARS for 99.99% of its $_GET and $_POST variables anyway. So, I would say that this is 'secure' - if there's a problem then there's a bug in the osc code, and this bug exists regardless of the register globals setting.

 

By far the biggest change that the patch addresses is the handling of session variables, and in this respect it does take a bit of a "make all session vars global" approach. However, for a variable to be present in the session, I was relying on OSC to put it there, so if there is a security problem with this then it lies with the osc logic and not with any register globals issue.

 

By the way, to address a previous post, and something that I have seen a few times previously, can I say that this contribution was NOT written as a bodge / hack to get osc working on a shared server which I had no control over. It was specifically written so that I could use OSC on a server that I chose to have register globals disabled on. I would not have used OSC at all if I could not have disabled register globals, so writing this was a necessity for me.

 

Rich.

Link to comment
Share on other sites

Archived

This topic is now archived and is closed to further replies.

×
×
  • Create New...