Jump to content
  • Checkout
  • Login
  • Get in touch

osCommerce

The e-commerce.

[contribution] Security Pro - Querystring protection against hackers.


FWR Media

Recommended Posts

Although this subject has a post elsewhere I was asked to put it up as a contribution especially as now we have some solid usage suggesting no major issues.

 

Quite recently I was involved in a topic related to customer_testimonials contribution where the "hacking world" had been made aware of an opportunity to hack osCommerce via a vulnerability in the querystring ($_GET/$HTTP_GET_VARS).

 

Our response was to "cleanse" the incoming $_GET/$HTTP_GET_VARS. However this approach is a losing game as with security it never makes sense to run around trying to sure up contributions individually. So I've been looking at this on "another forum" and have come up with a solution that I would now call beta.

 

The concept here (not a new one) is to totally sanitise the incoming ($_GET/$HTTP_GET_VARS) at source (the top of catalog/includes/application_top.php) then to sanitise $_REQUEST by $_REQUEST = $_GET + $_POST (Yes we lost $_COOKIE).

 

By "sanitise" they key here is that we are ALLOWING certain characters to exist in the querystring NOT trying to clean away some dirty ones.

 

The danger here of course is that we inadvertently remove a character that is required for a legitimate osCommerce function.

 

After much testing allowed characters are as follows: -

a-z

A-Z

0-9

.(dot)

-(hyphen)

_(underscore)

{}

space (needed for search)

% (To avoid breaking urlencoded strings used by payment systems) - Thanks perfectpassion.

 

We are zealously cleaning here so there is always a risk that some contibution may introduce to the querystring a character that is not allowed, so please ensure that you fully test that all your payment systems etc. are functioning correctly.

 

Upgrade: This package has a minor change to the code/positioning in catalog/includes/application_top.php (To allow admin On/Off). Plus an install script for the admin settings.

 

Hope it keeps you all safe.

 

Contribution http://addons.oscommerce.com/info/5752

Edited by Babygurgles
Link to comment
Share on other sites

Ok we have a new problem to resolve.

 

In the customers_testimonials hacking incident the hack still exists and the reason it exists is that customer_testimonials doesn't use $_GET['testimonials_id'] or $HTTP_GET_VARS['testimonials_id'].

 

It uses the $GLOBAL (Don't you just love register_globals) and in the file uses just $testimonials_id. This hasn't been cleansed by FWR Security Pro and so the hack is still at large.

 

The following is a new work in progress to add the variables added to $GLOBALS via the querystring to our cleansing operations.

 

Please try out the modifications below, and please feed back your thoughts and experiences so we can move this forward from "work in progress" to a version.

 

The below assumes that FWR Security Pro is already installed.

 

catalog/includes/application_top.php

 

Find:

 

 

$_REQUEST = $_GET + $_POST;

 

 

Add below:

 

fwr_clean_global($_GET); // Change the $GLOBALS value to the cleaned value

 

 

catalog/includes/functions/security.php

 

Add the following function to the bottom of the file before the closing ?>

 

 

function fwr_clean_global($get_var) {
 foreach ($get_var as $key => $value)
 ( isset($GLOBALS[$key]) ? $GLOBALS[$key] = $get_var[$key] : NULL );
}

 

How to test?

 

Put the following immediately after the // END - FWR Media Security Pro in application_top.php

 

// REMOVE ME
( isset($fwrtest) ? die($fwrtest) : NULL );
// REMOVE ME

 

Go to your shop with the url

 

www(dot)myshop.com/index.php?fwrtest=[w](o)%3Cr%3Ek|i*n^g

 

If it says just "working" then it's .. errm .. working.

Edited by FWR Media
Link to comment
Share on other sites

Ok we have a new problem to resolve.

 

In the customers_testimonials hacking incident the hack still exists and the reason it exists is that customer_testimonials doesn't use $_GET['testimonials_id'] or $HTTP_GET_VARS['testimonials_id'].

 

It uses the $GLOBAL (Don't you just love register_globals) and in the file uses just $testimonials_id. This hasn't been cleansed by FWR Security Pro and so the hack is still at large.

 

The following is a new work in progress to add the variables added to $GLOBALS via the querystring to our cleansing operations.

 

Please try out the modifications below, and please feed back your thoughts and experiences so we can move this forward from "work in progress" to a version.

 

The below assumes that FWR Security Pro is already installed.

 

catalog/includes/application_top.php

 

Find:

$_REQUEST = $_GET + $_POST;

Add below:

 

fwr_clean_global($_GET); // Change the $GLOBALS value to the cleaned value

catalog/includes/functions/security.php

 

Add the following function to the bottom of the file before the closing ?>

function fwr_clean_global($get_var) {
 foreach ($get_var as $key => $value)
 ( isset($GLOBALS[$key]) ? $GLOBALS[$key] = $get_var[$key] : NULL );
}

 

How to test?

 

Put the following immediately after the // END - FWR Media Security Pro in application_top.php

 

// REMOVE ME
( isset($fwrtest) ? die($fwrtest) : NULL );
// REMOVE ME

 

Go to your shop with the url

 

www(dot)myshop.com/index.php?fwrtest=[w](o)%3Cr%3Ek|i*n^g

 

If it says just "working" then it's .. errm .. working.

 

 

This contribution sounds like must if you have customers_testimonials installed?

I take it your updated code does not affect anything else negatively but will help secure other contributions that use this "get" string hack?

Edited by none_uk
Link to comment
Share on other sites

This contribution sounds like must if you have customers_testimonials installed?
I take it your updated code does not affect anything else negatively but will help secure other contributions that use this "get" string hack?

 

I make no guarantees about its "effect" on other contributions on your site. It is up to you to test before adding anything to a live site.

 

This contribution attempts to secure the querystring at source, the individual contributions should still be written and secured in an appropriate manner.

Link to comment
Share on other sites

In fact the "work in progress" version (i.e. not the one in contributions) WILL break payment systems. I'm working on a file exclusion system atm.

Link to comment
Share on other sites

Attached is version 1.0.1 as discussed above, this version has no testing and should be considered ALPHA.

 

Cleans querystring created $GLOBALS. (i.e. would have killed the customer_testimonials hack)

 

Admin settings for excluding specific files if your e.g. payment system can't work with the strict cleansing.

 

This is added here as an attachment because it is in an alpha state, please try it out we need the feedback and testing.

 

If you do try/use this please feed back here.

 

Once sufficient testing has been done it should (perhaps in modified form) eventually be added as a contribution.

Edited by FWR Media
Link to comment
Share on other sites

I reckon this could be one of the essential contributions to install on Oscommerce and it's great that it's being developed - thanks Rob !

The more feedback the better.

I installed the updated code posted above,reverted my customer_testimonials page back to the original unsanitized version and tested the hack urls used on my Customer Testimonials page .... they received error codes.

So Security Pro protected my database from the sql injection from these known urls - these would otherwise have pulled the data out. When I switched off Security Pro in admin these same urls pulled the data out.When switched on again..error codes.So it worked on what I tried.

I may add that this is a test site otherwise I would not have changed the secure code now in my customer testimonials page. Securing the contributions should not be neglected,but it is reassuring that Security Pro is another line of defence.

The functionality of the site seems fine from tests so far and I also did a couple of demo orders with Paypal and 2checkout and both worked and were logged fine.

I only installed this a few hours ago,so will report back again later.

 

Chris

Link to comment
Share on other sites

Now that you can "exclude" important files like payment files, I would love some feedback on the potential removal of the % character from the preg_replace.

 

I really don't want it to be there so help me out here please :D

Edited by FWR Media
Link to comment
Share on other sites

Another test for those who are testing on local PC/test server and can't test payment systems etc. (This tests $GLOBALS cleansing)

 

Put the following immediately after the // END - FWR Media Security Pro in application_top.php

// REMOVE ME
( isset($fwrtest) ? die($fwrtest) : NULL );
// REMOVE ME

 

In admin>FWR Security Pro

 

>File Exclusions> put product_info.php

 

Turn File Inclusion On .. to false

 

Browse to a products details (product_info.php) add on to the querystring

 

 

(& or ?)fwrtest=[w](o)%3Cr%3Ek|i*n^g

 

You should get

[w](o)%3Cr%3Ek|i*n^g

 

Back to admin Turn File Inclusion On .. to true

 

Do same again.

 

You should get

working
Edited by FWR Media
Link to comment
Share on other sites

Hi, this may be OFFtopic and I appologise. How do you recognise an attack ?

 

I have been getting A LOT of requests like the following from one ip and I dont know it it is attacking me or something else.... I get in apache logs:

 

 

mydomain.com/CB376A--hp-cb376a-multifunctionala-m1005-a4/ ';var%20midStr%20=%20'/';var%20midStr%20=%20'/images/infobox/';var%20midStr%20=%20'/images/infobox/images/infobox/images/infobox/';var%20midStr%20=%20'/';var%20midStr%20=%20'/';var%20midStr%20=%20'/images/infobox/';var%20midStr%20=%20'/';var%20midStr%20=%20

 

 

what is this ??

Link to comment
Share on other sites

No not offtopic really it looks like a hack bot to me but in this example midStr is a Pascal command if I'm not mistaken.

 

If it's one IP then great .. block it through IP tables or if you don't control your server ask your server admin to block the IP.

 

I can't see that particular string having any effect on osC though.

Edited by FWR Media
Link to comment
Share on other sites

  • 2 weeks later...

I have installed the new version 1.0.1 in my Oscommerce ebook store today, previously people are hacking my files without paying through paypal. Now can I expect this does not happen again?

 

Thank you,

 

Harris

Link to comment
Share on other sites

I have installed the new version 1.0.1 in my Oscommerce ebook store today, previously people are hacking my files without paying through paypal. Now can I expect this does not happen again?

 

In a word NO

 

How are they hacking your files atm?

 

Security Pro attempts to clean the variables created by the querystring at source. It is a layer of defence but you still have to ensure that your files and the server are secure.

Link to comment
Share on other sites

In a word NO

 

How are they hacking your files atm?

 

Security Pro attempts to clean the variables created by the querystring at source. It is a layer of defence but you still have to ensure that your files and the server are secure.

 

But Still this problem persists in another way not through query string, I have a serious issue in my site where people are hacking the files with out paying with paypal. When I try to test this process I have ensured that the paypal payment system is not secured before going to paypal when I try with the specific success url I can see the download files in the page. I am worried about this unsecured payment process when I use paypal. Can you please help me to avoid this situation?

Link to comment
Share on other sites

But Still this problem persists in another way not through query string, I have a serious issue in my site where people are hacking the files with out paying with paypal. When I try to test this process I have ensured that the paypal payment system is not secured when I am going to paypal and try with the specific success url I can see the download files in the page. I am worried about this unsecured payment process when I use paypal. Can you please help me to avoid this situation?
Link to comment
Share on other sites

This thread is for Security Pro not a question and answer to solve the hacking of a payment system.

Link to comment
Share on other sites

But Still this problem persists in another way not through query string, I have a serious issue in my site where people are hacking the files with out paying with paypal. When I try to test this process I have ensured that the paypal payment system is not secured before going to paypal when I try with the specific success url I can see the download files in the page. I am worried about this unsecured payment process when I use paypal. Can you please help me to avoid this situation?

a lot of people have complained about this. it isn't a hack. it's simply the fact of somebody getting to checkout_confirmation & then manually typing checkout_process.php and getting the success page.

 

if i offered downloads, i would offer an alternative method of supplying the download. say, after the payment has been manually verified.

Link to comment
Share on other sites

a lot of people have complained about this. it isn't a hack. it's simply the fact of somebody getting to checkout_confirmation & then manually typing checkout_process.php and getting the success page.

 

if i offered downloads, i would offer an alternative method of supplying the download. say, after the payment has been manually verified.

 

Will the contribution "PayPal_Shopping_Cart_IPN" help resolve this issue? Please reply.

Link to comment
Share on other sites

Nice work!

 

Another one for the excludes list - redirect.php.

 

Product urls to external sites break with SecurityPro on but the passed url is checked by redirect.php against the legitimate url for the product so redirect.php can be safely excluded.

 

HTH

 

Chris

Please use forum for support rather than PM - PMs unrelated to my contributions will be ignored.

Google Site Search is your friend

My contributions: Tracking Module | PDF Customer Invoice | Subcategory textboxes

Link to comment
Share on other sites

Nice work!

 

Another one for the excludes list - redirect.php.

 

Product urls to external sites break with SecurityPro on but the passed url is checked by redirect.php against the legitimate url for the product so redirect.php can be safely excluded.

 

HTH

 

Chris

 

Thanks for responding Chris

 

This was actually known but imo that function should not exist. I just don't like the idea of any script that allows a redirect to offsite. Redirect.php has been exploited in the past.

Link to comment
Share on other sites

This was actually known but imo that function should not exist. I just don't like the idea of any script that allows a redirect to offsite. Redirect.php has been exploited in the past.

 

Couldn't agree more, though I have several clients reliant on the url redirecting. I think Harald's default query check should be sufficient to stop an exploit here (pls correct me if I'm wrong)

 

I agree with you about getting rid of % from the allowed characters. As you were asking for ideas, rather than get rid of it completely, would a new regexp of allowed encoded characters be any better, say allowing %20 for space but explicitly excluding %3C and %3E (<> definite no-nos)?

 

I appreciate this would be difficult in view of the different payment modules but just getting rid of the worst suspects would be an improvement IMHO.

 

Again, thanks for your contrib

 

Cheers

 

Chris

Please use forum for support rather than PM - PMs unrelated to my contributions will be ignored.

Google Site Search is your friend

My contributions: Tracking Module | PDF Customer Invoice | Subcategory textboxes

Link to comment
Share on other sites

I agree with you about getting rid of % from the allowed characters. As you were asking for ideas, rather than get rid of it completely, would a new regexp of allowed encoded characters be any better, say allowing %20 for space but explicitly excluding %3C and %3E (<> definite no-nos)?

 

This is very much in mind, however %3C isn't enough you need %253E as well because of double encoding.

str_replace(array('<', '%3C', '%253C', '>', '%3E', '%253E'), array('', '', '', '', '', ''), $get_var);

 

and then there's octals and then there's hexadecimal .. I'm still looking at these.

 

I appreciate this would be difficult in view of the different payment modules but just getting rid of the worst suspects would be an improvement IMHO.

 

This wont really be worthy of its name until it DOES start breaking more scripts. The idea should be that it sanitizes as much as possible and only "trusted" scripts get to be allowed through as an exception.

 

The % should definately be removed from the preg_replace otherwise all you need do (well nearly) is urlencode a querystring to bypass cleansing. That or urldecode before cleansing.

Link to comment
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Unfortunately, your content contains terms that we do not allow. Please edit your content to remove the highlighted words below.
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...