Jump to content
  • Checkout
  • Login
  • Get in touch

osCommerce

The e-commerce.

Understanding the register_globals problem


djmonkey1

Recommended Posts

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?

Yes, I see what you're saying now. And thanks for your comments. Pleased to see that someone finds this stuff useful.

 

By the way, I'm about to post an update to the patch (maybe later tonight - I'll see). Nothing fantastic though so don't get all excited :-)

 

Rich.

Link to comment
Share on other sites

  • Replies 67
  • Created
  • Last Reply
Yes, I see what you're saying now. And thanks for your comments. Pleased to see that someone finds this stuff useful.

 

By the way, I'm about to post an update to the patch (maybe later tonight - I'll see). Nothing fantastic though so don't get all excited :-)

 

Rich.

 

I'll keep my eyes open for it ;)

Link to comment
Share on other sites

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.

 

yes, I know what it does technically but functionally it only enables osc to function with register globals off and as such it does not plug any security holes itself.

Treasurer MFC

Link to comment
Share on other sites

yes, I know what it does technically but functionally it only enables osc to function with register globals off and as such it does not plug any security holes itself.

Ah, I see what you're saying.

 

Well, to that I would pose the following questions...

 

1/ Are you ABSOLUTELY sure the base osc code does not contain one or more bugs that would/could be affected by variable injection via $_POST / $_GET ?

 

2/ Are you ABSOLUTELY sure all those lovely contributions you have patched in don't contain any bugs that could be exploited by variable injection ?

 

3/ If you run with register globals enabled then all the other PHP apps on your server will also be running with register globals enabled. Are you ABSOLUTELY sure none of those contains a bug that bla bla bla...

 

Get the point ? I could not say "yes" to any any of the above questions (especially (2) !!!). All these scenarios expose potential security risks, and if you don't KNOW that these things is secure and 100% correct then you need to assume that they are not.

 

Footnote: Ref. (3) above - Yes, I know you can use .htaccess to disable / enable register globals on a per-application basis but (A) this doesn't do anything to address points (1) and (2) and (B) using .htaccess is a security risk in itself. If you don't believe me, see the apache docs - .htaccess is a hack (I paraphrase the apache docs; I'm not mud-slinging) and was invented to get round a user's inability to get access to the main apache configuration on a shared server, and allowing it (its use has to be enabled from the main apache config) adds an extra security risk that one can do without (if one has the choice).

 

Rich.

Link to comment
Share on other sites

Ah, I see what you're saying.

 

Well, to that I would pose the following questions...

 

1/ Are you ABSOLUTELY sure the base osc code does not contain one or more bugs that would/could be affected by variable injection via $_POST / $_GET ?

There are thousands upon thousands of instances of osC running. The code is available to anyone who cares to peruse it.

 

If it was easily exploited we'd see hundreds and hundreds of posts here about this exploit or that.

 

Where are they?

Local: Mac OS X 10.5.8 - Apache 2.2/php 5.3.0/MySQL 5.4.10 • Web Servers: Linux

Tools: BBEdit, Coda, Versions (Subversion), Sequel Pro (db management)

Link to comment
Share on other sites

what the register globals-off has to do with this? you have this code for a get variable

 

  function link_get_variable($var_name)
 {
// Map global to GET variable
if (isset($_GET[$var_name]))
{
  $GLOBALS[$var_name] =& $_GET[$var_name];
}
 }

 

So now say

 

$cID = '5%20UNION%20ALL%20SELECT something from the database';

when I call the link_get_variable with $cID as parameter, we will have the injection global aren't we? Unless I filter it, it maintains the original security risk.

Link to comment
Share on other sites

There are thousands upon thousands of instances of osC running. The code is available to anyone who cares to peruse it.

 

If it was easily exploited we'd see hundreds and hundreds of posts here about this exploit or that.

 

Where are they?

What's your point? You haven't been burgled yet, so you still leave the door open?

 

...and I know of at least two sets of successful attacks on osc installations. And you really don't have to look very far in these forums to find posts from people who have been hacked. Ok, not all of them have been hacked directly through OSC, but that's the thing about computer security; attacks are often from unexpected angles.

 

Should I wait until my web site has been hacked before I put counter-measures in place? Should I put-off plugging a known security risk with a simple and known working solution because I'm not aware of where the bugs are? OSC contains bugs. It's software - what do you expect?! Just because you don't know where those bugs are doesn't mean you shouldn't protect as best you can against exploits. Maybe I should set all my file permissions to 777 because it makes life easy? That's always a good one :-)

 

You are being amazingly complacent. But it's up to you, of course.

 

Rich.

Link to comment
Share on other sites

what the register globals-off has to do with this? you have this code for a get variable

...

$cID = '5%20UNION%20ALL%20SELECT something from the database';

when I call the link_get_variable with $cID as parameter, we will have the injection global aren't we? Unless I filter it, it maintains the original security risk.

No, it does not in itself contain a security risk.

 

If you have some code that uses the variable $_GET['cID'] (which is what the example you present is doing) then it's up to the application to make sure that there is nothing horrible in that variable and deal with it as it sees fit (and I think osc is probably reasonably good in this respect). This has got nothing at all to do with whether register globals is enabled or not; the risk exists regardless. It's everything to do with good coding practice.

 

When I refer to variable injection, I mean injection of variables that the code is NOT expecting to be set from outside (ie - from the client). It's THIS sort of variable injection that switching register globals off protects against.

 

Rich.

Link to comment
Share on other sites

Oh, okay then.

 

It seems to me that your trying to "GET" something that you've "POST"'ed, or am I wrong?

 

Exactly- I did it. Everything works without patching, hacks, workarounds, whatever you want to call it.

 

For the record, the only thing I ever referred to as a hack, in this thread, was

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

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

Ok I see, I read point-1 from your earlier post where you mentioned about the _GET and _POST arrays.

1/ Are you ABSOLUTELY sure the base osc code does not contain one or more bugs that would/could be affected by variable injection via $_POST / $_GET ?

That's the reason I gave the example.

Link to comment
Share on other sites

Ah, I see what you're saying.

 

Well, to that I would pose the following questions...

 

1/ Are you ABSOLUTELY sure the base osc code does not contain one or more bugs that would/could be affected by variable injection via $_POST / $_GET ?

 

2/ Are you ABSOLUTELY sure all those lovely contributions you have patched in don't contain any bugs that could be exploited by variable injection ?

 

3/ If you run with register globals enabled then all the other PHP apps on your server will also be running with register globals enabled. Are you ABSOLUTELY sure none of those contains a bug that bla bla bla...

 

Get the point ? I could not say "yes" to any any of the above questions (especially (2) !!!). All these scenarios expose potential security risks, and if you don't KNOW that these things is secure and 100% correct then you need to assume that they are not.

 

Footnote: Ref. (3) above - Yes, I know you can use .htaccess to disable / enable register globals on a per-application basis but (A) this doesn't do anything to address points (1) and (2) and (B) using .htaccess is a security risk in itself. If you don't believe me, see the apache docs - .htaccess is a hack (I paraphrase the apache docs; I'm not mud-slinging) and was invented to get round a user's inability to get access to the main apache configuration on a shared server, and allowing it (its use has to be enabled from the main apache config) adds an extra security risk that one can do without (if one has the choice).

 

Rich.

 

yes, but all register globals off does is disallow certain potentially insecure coding practices.

As such that is a good thing, certainly with lalaland contributions out there.

 

Still, all your contribution does (as good and innovative as it may be) is allow base osc to function with that setting.

 

So I am not saying that that contribution is not good or not necessary or irrelevant or a hack or..., I merely dispute your claim that that contribution plugs a security hole as it does nothing of the sort.

Treasurer MFC

Link to comment
Share on other sites

... I merely dispute your claim that that contribution plugs a security hole as it does nothing of the sort.

Fair enough - I hear what you are saying.

 

Personally, I consider it a security risk though, and ok if you don't want to consider it a hole, it's certainly a POTENTIAL hole; I'm sure there are more than a handful of dodgy contributions out there that could be exploited in this way.

 

It wasn't too long ago when quite a few osc sites got hacked because they were running with register globals enabled and this opened up a security hole in some BB s/w that was running on the same server. Yes, the fault was with the BB s/w, but it was opened up by osc's need to have register globals enabled. You have to take a holistic view of these things; you can't view stuff in isolation.

 

Either way, I would never run with register globals enabled, regardless of which software I was running; it's a risk that need not be there. There's lots of potential holes that can be plugged; running your web server in a chroot / jail, being very tight on file permissions / users, etc etc... Not being able to identify a specific threat doesn't mean that these things should not be addressed.

 

Maybe we just differ on how we view the seriousness of these things; I tend towards the paranoid and I know I get a bit anal about this stuff, but so far this attitude has not let me down :-)

 

I'll shut up now before I talk more crap.

 

Rich.

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?

 

Actually, there is one small problem with steps 1-3, and it is that the selected value is not being displayed once the option is submitted, it just goes back to saying "Top" or "Choose a Product" or whatever. Which is fine because otherwise the form works, but it would be nice if the selected options remained selected.

 

Here is step two:

 

	// Step 2: Choose Product
	if(($_POST['step'] > 1) && ($_POST['add_product_categories_id'] > 0))

	{
		echo '<tr class="dataTableRow"><form action=' . $_SERVER['PHP_SELF'] .'?oID=' . $_GET['oID'] . '&action=' . $_GET['action'] . ' method="POST">' . "\n";
		print "<td class='dataTableContent' align='right'><b>" . ADDPRODUCT_TEXT_STEP . " 2: </b></td>\n";
		if (is_array($_POST['add_product_products_id'])) $selected = $_POST['add_product_products_id']; else $selected=""; 
		echo '<td class="dataTableContent" valign="top"><select name="add_product_products_id" onChange="this.form.submit();">';
		$ProductOptions = "<option value='0'>" .  ADDPRODUCT_TEXT_SELECT_PRODUCT . "\n";
		asort($ProductList[$_POST['add_product_categories_id']]);
		foreach($ProductList[$_POST['add_product_categories_id']] as $ProductID => $ProductName)
		{
		$ProductOptions .= "<option value='$ProductID'> $ProductName\n";
		}
		$ProductOptions = str_replace("value='$add_product_products_id'","value='$add_product_products_id' selected", $ProductOptions);
		print $ProductOptions;
		print "</select></td>\n";
		echo '<input type="hidden" name="add_product_categories_id" value=' . $_POST['add_product_categories_id'] . '>';
		echo '<input type="hidden" name="step" value="3">' . "\n";
		print "<td class='dataTableContent'>" . ADDPRODUCT_TEXT_STEP2 . "</td>\n";
		print "</form></tr>\n";
		print "<tr><td colspan='3'> </td></tr>\n";
	}

 

I've added in

if (is_array($_POST['add_product_products_id'])) $selected = $_POST['add_product_products_id']; else $selected="";

but as you can see it's not being used. I've tried using it what seems like a million different ways but nothing has worked so far.

 

It seems like the str_replace function should do what I'm looking for but I haven't gotten any satisfaction from that either.

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

Actually, there is one small problem with steps 1-3, and it is that the selected value is not being displayed once the option is submitted, it just goes back to saying "Top" or "Choose a Product" or whatever. Which is fine because otherwise the form works, but it would be nice if the selected options remained selected.

 

Here is step two:

 

	// Step 2: Choose Product
	if(($_POST['step'] > 1) && ($_POST['add_product_categories_id'] > 0))

	{
		echo '<tr class="dataTableRow"><form action=' . $_SERVER['PHP_SELF'] .'?oID=' . $_GET['oID'] . '&action=' . $_GET['action'] . ' method="POST">' . "\n";
		print "<td class='dataTableContent' align='right'><b>" . ADDPRODUCT_TEXT_STEP . " 2: </b></td>\n";
		if (is_array($_POST['add_product_products_id'])) $selected = $_POST['add_product_products_id']; else $selected=""; 
		echo '<td class="dataTableContent" valign="top"><select name="add_product_products_id" onChange="this.form.submit();">';
		$ProductOptions = "<option value='0'>" .  ADDPRODUCT_TEXT_SELECT_PRODUCT . "\n";
		asort($ProductList[$_POST['add_product_categories_id']]);
		foreach($ProductList[$_POST['add_product_categories_id']] as $ProductID => $ProductName)
		{
		$ProductOptions .= "<option value='$ProductID'> $ProductName\n";
		}
		$ProductOptions = str_replace("value='$add_product_products_id'","value='$add_product_products_id' selected", $ProductOptions);
		print $ProductOptions;
		print "</select></td>\n";
		echo '<input type="hidden" name="add_product_categories_id" value=' . $_POST['add_product_categories_id'] . '>';
		echo '<input type="hidden" name="step" value="3">' . "\n";
		print "<td class='dataTableContent'>" . ADDPRODUCT_TEXT_STEP2 . "</td>\n";
		print "</form></tr>\n";
		print "<tr><td colspan='3'> </td></tr>\n";
	}

 

I've added in

if (is_array($_POST['add_product_products_id'])) $selected = $_POST['add_product_products_id']; else $selected="";

but as you can see it's not being used. I've tried using it what seems like a million different ways but nothing has worked so far.

 

It seems like the str_replace function should do what I'm looking for but I haven't gotten any satisfaction from that either.

 

 

Only took a quick look and not sure what you are trying to achieve with $selected but what about the below

 

foreach($ProductList[$_POST['add_product_categories_id']] as $ProductID => $ProductName)
{
if(isset($_POST[$ProductID]))
{
$ProductOptions .= "<option value='$ProductID' selected='selected' /> $ProductName\n";
}else $ProductOptions .= "<option value='$ProductID' /> $ProductName\n";
}

Link to comment
Share on other sites

**CORRECTION**

 

foreach($ProductList[$_POST['add_product_categories_id']] as $ProductID => $ProductName)
{
if(isset($_POST[$ProductID]))
{
$ProductOptions .= '<option value="' . $ProductID . '" selected="selected" />' . $ProductName . "\n";
}else $ProductOptions .= '<option value="' . $ProductID . '" />' . $ProductName . "\n";
}

Link to comment
Share on other sites

**CORRECTION**

 

foreach($ProductList[$_POST['add_product_categories_id']] as $ProductID => $ProductName)
{
if(isset($_POST[$ProductID]))
{
$ProductOptions .= '<option value="' . $ProductID . '" selected="selected" />' . $ProductName . "\n";
}else $ProductOptions .= '<option value="' . $ProductID . '" />' . $ProductName . "\n";
}

 

That didn't work, so I tried

foreach($ProductList[$_POST['add_product_categories_id']] as $ProductID => $ProductName)
		{
			if (isset($_POST['add_product_products_id']))
				  {
						  $ProductOptions .= '<option value="' . $ProductID . '" selected="selected" />' . $ProductName . "\n";
					}  
					else $ProductOptions .= '<option value="' . $ProductID . '" />' . $ProductName . "\n";
		   }

 

This way, once you've selected a product from the list, the correct data is posted but the last item in the list is the one that's displayed in subsequent steps, which is pretty bizarre.

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

This way, once you've selected a product from the list, the correct data is posted but the last item in the list is the one that's displayed in subsequent steps, which is pretty bizarre.

 

Got it to work by modifying the str_replace function:

 

   if(isset($_POST['add_product_products_id'])){
	 $ProductOptions = str_replace("value='" . $_POST['add_product_products_id'] . "'", "value='" . $_POST['add_product_products_id'] . "' selected=\"selected\"", $ProductOptions);
	   }

 

Did the same thing for step 3, now the only bugaboo still facing me is to get step 1 to work the same way.

 

Right now I've got 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">';
		if (isset($_POST['add_product_categories_id'])) {
		$add_product_categories_id = str_replace("value=\"0\" selected=\"selected\"", "value=\"0\"", $add_product_categories_id);
		$add_product_categories_id = str_replace("value='" . $_POST['add_product_categories_id'] . "'", "value='" . $_POST['add_product_categories_id'] . "' selected=\"selected\"", $add_product_categories_id);
		 }
		echo ' ' . tep_draw_pull_down_menu('add_product_categories_id', tep_get_category_tree(), $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";

 

Without this, on select the first option value (option value="0") is set as "selected" so that's where the first str_replace comes from. However, using this code none of the options appear as "selected" and so "Top" still appears in the selection box once a category has been selected.

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

BTW-

 

According to the minutes of the November 11-12 PHP developers meeting, register_globals wil not even be an option in PHP 6.

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

Archived

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

×
×
  • Create New...