SQL injection Vulnerability

[voided]

Just got this in my bugtraq inbox...

 

For us that still run 2.2-MS1, it would be really great to know if this can be "patched" easily to correct the problem. Redoing our site from scratch would take quite some time :(

 

Vendor  : osCommerce

URL    : http://www.oscommerce.com

Version : osCommerce 2.2-MS1 / Older Versions?

Risk    : SQL Injection Vulnerability

 

 

Description:

osCommerce is an online shop e-commerce solution under on going

development by the open source community. Its feature packed

out-of-the-box installation allows store owners to setup, run, and

maintain their online stores with minimum effort and with absolutely

no costs or license fees involved.

 

 

Problem:

osCommerce is vulnerable to SQL Injection vulnerability in the

create_account_process.php file. osCommerce 2.2-MS1 is the affected

version, and older versions are likely to be affected as well. The

attack can be carried out by a malicious user who is registering an

account. All one has to do to exploit the vulnerability is save the

registration form offline. After doing so an attacker can edit the

"country" field to include SQL.

 

 

Solution:

The vendor was not contacted as the problem seems to have been resolved

in osCommerce 2.2-MS2 Users are advised upgrade to the current milestone

release http://www.oscommerce.com/downloads

 

 

Credits:

Credits go to JeiAr of the GulfTech Security Research Team.

http://www.gulftech.org

[Guest]

If you search the security focus site for sql injection, you will see what they are talking about, and be able to figure out what to put where in the code. As I didnt start with MS1, this was moot for me, however for PHPBB forums, it did and I have about 10 of those, so had to change 2 lines of code, and away it went!

[voided]

great idea.

 

here's the link : http://www.securityfocus.com/archive/1/347582

[voided]

the link to the script that checks for the vuln. confirms it

 

"Host Is Vulnerable!" crap... lets see if i can find out how to fix it

[Matt L]

For people running MS1 and earlier, if you're interested in a quick and dirty fix, you can manually strip some characters out of the country field prior to its use in the sql statement in order to prevent this specific attack.

 

I should state right now that this by no means the proper way to eliminate these types of effects on a site-wide basis, but it will quickly mitigate this *specific* problem until you find a better solution. The proper way to do this is to filter all submitted content prior to using it at all. MS2 I believe does this already and there are many, many ways, even built into php, to prevent this type of attack.

 

Having said that, for people that want to "open a file and make this problem go away right now", you can do the following.

 

In create_account_process.php, find this section of code:

 

    tep_db_perform(TABLE_CUSTOMERS, $sql_data_array);

   $customer_id = tep_db_insert_id();

   $sql_data_array = array('customers_id' => $customer_id,
                           'address_book_id' => 1,
                           'entry_firstname' => $firstname,
                           'entry_lastname' => $lastname,
                           'entry_street_address' => $street_address,
                           'entry_postcode' => $postcode,
                           'entry_city' => $city,
                           'entry_country_id' => $country);

 

and insert some perl (of your choice) to strip out the malicious content. For instance:

 

    tep_db_perform(TABLE_CUSTOMERS, $sql_data_array);

   $customer_id = tep_db_insert_id();

// Strip out single quotes
       $country =~ s/\'//g;

   $sql_data_array = array('customers_id' => $customer_id,
                           'address_book_id' => 1,
                           'entry_firstname' => $firstname,
                           'entry_lastname' => $lastname,
                           'entry_street_address' => $street_address,
                           'entry_postcode' => $postcode,
                           'entry_city' => $city,
                           'entry_country_id' => $country);

 

The exploit provided should now return the following:

 

[matt@store]:~$ perl ossqlin.pl  www.mystore.net
Trying www.mystore.net ....

Host NOT Vulnerable

 

In reality, many characters should be stripped, not just single quotes, of course. Semicolons, double quotes, it's all dependent upon your backend code and what you are and are not escaping for legitimate input purposes.

 

Instead of blacklisting characters, in situations like this, whitelisting characters is a better solution. Of course, in a site-wide code scenario, a two-stage approach of a general site-wide blacklist of characters that should *never* be allowed, followed by a more specific whitelist approach per-input-field or form, is the most secure. It may not be the most feasible solution, but for paranoid folk like myself, it helps us sleep at night. :)

 

In any event, I again state to please not use this as a permanent fix and to explore better options for input validation for your installation. There's a ton of ways to fix problems like this temporarily, this is one of them, and it is specific to the bugtraq exploit in question. There are almost definitely additional, nearly identical exploits in the MS1 code. Updating to MS2 (Hi pot, I'm kettle, nice to meet you) would be a good choice to avoid these problems in the future. :)

 

Caveats: No guarantee this will work, your mileage may vary, blah blah blah.

 

-Matt

[Matt L]

Oh, and if you want to be extra paranoid, just insert the line stripping out the quote right at the assignment stage (this is near the top of the file):

 

  $zone_id = tep_db_prepare_input($HTTP_POST_VARS['zone_id']);
 $state = tep_db_prepare_input($HTTP_POST_VARS['state']);
 $country = tep_db_prepare_input($HTTP_POST_VARS['country']);

// new code here
 $country =~ s/\'//g;
// end new code

 $error = false; // reset error flag

 

The $country variable is used several times prior to the insert statement, although it's wrapped inside tep_db_input (meaning it's run through addslashes prior to being used).

 

I'd recommend stripping out much more than just the single quote, but again, this fix is specific to the bugtraq exploit. Read my prior post for the extended rant. :)

 

This of course brings up the point of just running addslashes (or the like) on all the form submitted data anyway prior to its being inserted, but that's another story. I've ranted enough about "real" input validation for one day. :)

 

Caveats pt 2: May not work at all, may blow up your store, eat your kids, etc. Use at your own risk.

 

-Matt

[Matt L]

I apologize, this only works with perl modifications on some carts, and not just PHP as most MS1 people are running.

 

Putting together php-based fix now, back in a moment.

 

-Matt

[voided]

matt i (we) appreciate what you are doing

 

thanks for the quick fix :D

[Guest]

In PHP, replace $country =~ s/\'//g; with

$country = preg_replace("/\'/", "", $country);

to get the same effect. Doesn't tep_db_prepare_input already do something like this?

 

Hth,

Matt

[hpqnet]

I am running 2.2 ms2 and have someone setting up a hello [email protected] account and staying in the accout/address_book areas ... It is someone trying sql injection I am sure. I have change the user password via db and yet they come back and work forever trying the old account then just setup another with hello2 ... funny yet serious. Does anyone know of any areas in ms2 that needs to be fixed?

[voided]

checking...

 

but if it doesn't i think we know what to do :P

[hpqnet]

what? What do I do to make certain im safe with ms2? I ran the perl script and it says NOT vulnerable. BUT this user keeps coming back almost daily, so I am just curious if they are trying to find a new way in, or have found something. I do see in the error_log where they are trying to get to the create_account_process.php file, which does not exist in ms2.

 

Any ideas? or am I jus being over worried?

[Matt L]

Looking into this, the first SQL statement does have tep_db_input wrapping the country variable, yet it still breaks. It's odd, if you wrap addslashes around it prior to being used in SQL at all, i.e.: "$country = addslashes($country);" then send it into SQL, the problem goes away. So addslashes is being run twice. It's peculiar, but it works. I'm sure it's something obvious.

 

I haven't touched OSCommerce code in 6 months or so except for a patch here and there, so I'm a bit out of the loop. I'm sure there's a logical reason for this, I just didn't have time to look into it.

 

I'd say just run the regular expression and pull out the characters.

 

$country = preg_replace("/\'/", "", $country);

 

...should work, as iiinetworks said, that's the php equivalent of the perl I stated earlier. Although this is definitely a hack, don't rely on it longer than you have to.

 

Still looking into it...

 

-Matt

[voided]

matt

 

functions/database.php

 

  function tep_db_prepare_input($string) {
   if (is_string($string)) {
     return trim(stripslashes($string));
   } elseif (is_array($string)) {
     reset($string);
     while (list($key, $value) = each($string)) {
       $string[$key] = tep_db_prepare_input($value);
     }
     return $string;
   } else {
     return $string;
   }
 }

 

to

 

 function tep_db_prepare_input($string) {
   if (is_string($string)) {
     $string = preg_replace("/\'/", "", $string);
     return trim(stripslashes($string));
   } elseif (is_array($string)) {
     reset($string);
     while (list($key, $value) = each($string)) {
       $string[$key] = tep_db_prepare_input($value);
     }
     return $string;
   } else {
     return $string;
   }
 }

 

 

seems to work. what do you think?

[hpqnet]

I did have my ISP block the IP address from accessing the website, but they can only do this as a temporary fix, and will have to lift the blockage due to legalities and lack of evidence... So i am trying to confirm I am safe before they lift the block...

 

Thanks

[Matt L]

Voided:

 

Heart's in the right place, although I'm uncomfortable making that change on a global scale.

 

Sometimes you need single quotes, in product names, in *real* names (customers), such as D'Angelo or something. Single quotes should be escaped and allowed into the database as necessary, but again, on a whitelist level.

 

If you arbitrarily start eliminating single quotes from input fields on a global basis, you could introduce more problems than you're fixing.

 

Realistically, the whole darn process would need to be revamped, which I hear has been done in MS2, although I haven't even looked at it. Right now, you just have to weigh your options.

 

For the moment, since the amount of fields people can insert single quotes into is relatively small, it may make sense to whitelist them on an individual basis, then simply strip them out of anything else right at the time the form is submitted. This would only require small modifications of a few files...

 

Hrm. Maybe we should put together an MS1 SQL Injection patch kit. Although doing a code audit isn't really something I want to do right now, if this is such a big problem, it might be worth doing. It'd take less than a day, although MS2 probably already has a lot of those fixes in place...

 

Thoughts?

 

-Matt

[voided]

matt you are right about the "global" change, it would be better to go on a selective method right now.

 

thanks for the tips i'll get working on it right away :)

[voided]

btw i'm using this :

 

$country = str_replace(";",'', str_replace("=",'', str_replace("'",'', str_replace('"','', $country))));

 

if anyone wants to convert it to preg then please go ahead, because i'm not really familiar with regexp at the moment :D

 

 

basically i only added this check on "create_account_process.php" and "account_edit_process.php" .... oh and the login.php too

$email_address = str_replace(";",'', str_replace("=",'', str_replace("'",'', str_replace('"','', $email_address))));

 

 

if anyone finds any other spots where to apply it then please point them out.

[wolter]

Hi All,

 

I just received a mail from my hosting provider telling me aboout the osCommerce security issue regarding mysql injections (whatever that meaybe, I?m a PHP newbee).

 

It?s interesting to see al the different solutions, but I kinda lost track of all that. Can someone who know?s what he?s doing summarize the changes that need to be made? It would be of great help to me and all other non PHP wizards out there :)

 

Kind regards

 

 

 

 

Wolter

[voided]

you should ask them to run this test : http://www.gulftech.org/vuln/ossqlin.txt

 

to see if your oscommerce version is affected and if it is well then you should apply the changes we have covered in here.

[hpqnet]

I am running ms2 and ran the script and it tells me im Not vun to the injections, HOWEVER this is what I get from one of the hacker accounts on my site. When I pull up the account via admin customers

 

Warning: Variable passed to reset() is not an array or object in /home/virtual/site83/fst/var/www/html/admin1/includes/classes/object_info.php on line 17

 

Warning: Variable passed to each() is not an array or object in /home/virtual/site83/fst/var/www/html/admin1/includes/classes/object_info.php on line 18

 

 

What is he doing???

[Guest]

Try patching includes/functions/general.php instead, guys. The vulnerability demonstrated in ossqlin.pl is in tep_get_zone_name(). There's no input validation on $country_id. Either change it to tep_db_input($country_id) or (int)$country_id, like this:

 

  function tep_get_zone_name($country_id, $zone_id, $default_zone) {
   $zone_query = tep_db_query("select zone_name from " . TABLE_ZONES . " where
zone_country_id = '" . (int)$country_id . "' and zone_id = '" . $zone_id . "'");
   if (tep_db_num_rows($zone_query)) {
     $zone = tep_db_fetch_array($zone_query);
     return $zone['zone_name'];
   } else {
     return $default_zone;
   }
 }

 

There are probably a lot more of these floating around in pre-MS2 code. Happy hunting. :)

[Guest]

(As long as you're in there fixing stuff, you might want to (int)$zone_id, too. You get the idea.)

[voided]

should have thought about it :/

 

 

thanks for the tip Mike

[Harald Ponce de Leon]

An announcement has been made in the latest weekly summary report which can be read here:

 

http://www.oscommerce.com/forums/index.php?showtopic=70525

 

An update package is available which describes how to update 2.2 Milestone 1 installations.