[contribution] Security Pro - Querystring protection against hackers.

[♥FWR Media]

Tougher security.php

 

A new catalog/includes/functions/security.php has been added to contributions.

 

The changes are minimal but it makes a big difference.

 

Changes:

% character removed

$get_var is urldecoded before the preg_replace strips bad characters.

 

No changes are required of the contribution just directly replace catalog/includes/functions/security.php

 

Please note: Unlike other contributions, this one will break more things the better it gets.

 

Odd sounding I know but it is the case. Now that this is urldecoding and is missing the % character a lot more scripts, payment modules etc will fail .. this is a GOOD thing. By all means exclude your broken payment module from cleansing by security Pro .. however, I wouldn't advise doing the same for a less important contribution .. why not see where it's stopped by this script and change it so that it doesn't use bad characters in the querystring.

 

Most important: Test fully your important systems after adding this . .especially payment/shipping etc.

 

As usual I need feedback.

 

Thanks to perfectpassion for continuing to help me test this alongside his PROTX payment module (which I use myself by the way).

 

 

Please try to think along the following lines:-

 

If Security Pro breaks a feature/function

 

1) Try to remove the need for the feature to use bad characters (Stay here as long as you can)

 

2) Exclude the file from cleansing only if you really have to. (Should be critical operations only like payment where you have no control over the incoming querystring)

 

3) NEVER alter the preg_replace or other functions in security.php unless improving it (making it tougher).

 

Note: With this contribution the usual "Yaaay I got it to work by removing XXX from security.php" = you broke your security!

[Dennisra]

Thanks for the update.

 

How, exactly do I exclude a file from cleansing?

Thanks!

 

Tougher security.php.

By all means exclude your broken payment module from cleansing by security Pro

2) Exclude the file from cleansing only if you really have to.

[♥FWR Media]

Thanks for the update.

 

How, exactly do I exclude a file from cleansing?

Thanks!

 

If you have the latest version there is an admin setting for this.

[Dennisra]

If you have the latest version there is an admin setting for this.

 

Best I can tell I have the latest version. The contribution has two files available available for download:

SecurityPro1.0.zip which I have installed and security.zip and I updated the

catalog/includes/functions/security.php file today.

 

In admin under configuration I have FWR Security Pro.The only option there is to enable true or false. I am sure I am missing something. Was there another database update that isn't in the last secuirty.zip file? Any assistance would be appreciated.

[♥FWR Media]

Best I can tell I have the latest version. The contribution has two files available available for download:

SecurityPro1.0.zip which I have installed and security.zip and I updated the

catalog/includes/functions/security.php file today.

 

In admin under configuration I have FWR Security Pro.The only option there is to enable true or false. I am sure I am missing something. Was there another database update that isn't in the last secuirty.zip file? Any assistance would be appreciated.

 

That is not the latest version .. see contributions.

[Dennisra]

That is not the latest version .. see contributions.

 

I give up. May I ask for a link to the newsest version? This is all I can find http://addons.oscommerce.com/info/5752 and as you said that's not the latest version.

Thanks for your help.

[♥FWR Media]

I give up. May I ask for a link to the newsest version? This is all I can find http://addons.oscommerce.com/info/5752 and as you said that's not the latest version.

Thanks for your help.

 

My apologies it is not up as a contribution yet see post 6

 

http://www.oscommerce.com/forums/index.php?s=&...t&p=1210572

[Dennisra]

My apologies it is not up as a contribution yet see post 6

 

http://www.oscommerce.com/forums/index.php?s=&...t&p=1210572

 

I totally missed that post. Thank you!

[♥FWR Media]

The 1.0.2 version is now uploaded to contributions.

[♥valerif]

thanks for the contributions

this is what i was looking for

 

i installed it and see this problem

Fatal error: Call to undefined function: fwr_clean_global() in /.../includes/application_top.php on line 88

 

if i disable the line from the application top code it works fine

 

perhaps you could have a suggestion what this could be

 

thanks

valeri

[♥FWR Media]

thanks for the contributions

this is what i was looking for

 

i installed it and see this problem

Fatal error: Call to undefined function: fwr_clean_global() in /.../includes/application_top.php on line 88

 

if i disable the line from the application top code it works fine

 

perhaps you could have a suggestion what this could be

 

thanks

valeri

 

Sounds like you have an old catalog/includes/functions/security.php

 

The latest version has a function fwr_clean_global()

[♥valerif]

Sounds like you have an old catalog/includes/functions/security.php

 

The latest version has a function fwr_clean_global()

yes you were right

the stramge thing is that i think i did download the new package

thanks for the help

valerif

[Guest]

FWR Media,

 

I just noticed this contribution affected my newsletter contributions, where I allow people to remove themselves from lists.

 

It strips the @ from the:

[email protected]

 

to fix this I edited security.php:

return preg_replace("/[^ {}a-zA-Z0-9_.-]/i", "", urldecode($get_var));

 

with:

return preg_replace("/[^ {}a-zA-Z0-9@_.-]/i", "", urldecode($get_var));

 

wondering if it's possible to incorporate this with a future version? :)

[Guest]

also added this to my admin panel.

 

top if admin/includes/application_top.php:

  require('../includes/functions/security.php');

 

never know when you get an angsty employee that wants to deface your site before he quits :)

[♥FWR Media]

FWR Media,

 

I just noticed this contribution affected my newsletter contributions, where I allow people to remove themselves from lists.

 

It strips the @ from the:

[email protected]

 

to fix this I edited security.php:

return preg_replace("/[^ {}a-zA-Z0-9_.-]/i", "", urldecode($get_var));

 

with:

return preg_replace("/[^ {}a-zA-Z0-9@_.-]/i", "", urldecode($get_var));

 

wondering if it's possible to incorporate this with a future version? :)

 

Changing the preg_replace is really the WORST possible thing to do, although having said that @ I can't see being any danger. I still think there is no reason for it to exist in the querystring. I would rather see the value passed as a replaceable character.

 

[url=http://mysite.com/index.php?querystring=rob-emailand-mysite.com]http://mysite.com/index.php?querystring=ro...land-mysite.com[/url]

 

then at the receiving end ..

 

str_replace('-emailand-', '@', $HTTP_GET_VARS['querystring']);

Edited March 7, 2008 by FWR Media

[Guest]

if the @ symbol is harmless, i don't see the point in adding additional code to sanitizing it.

 

the problem with this is that oscommerce stores the emails with the @ in tact. so eventually it's going to need to be in there anyway if you're searching to remove a specific address.. i haven't tried to see if the ascii bits of @ would work the same way.. but if you can't get hacked by somebody passing @ in a querystring i fail to see the point in sanitizing it, especially if security pro already only allows a-z, 09 and dots and dashes.. so before and after @ are already cleansed (and are cleansed inside the unsubscribe.php codes, as well) :)

 

str_replace('-emailand-', '@', $HTTP_GET_VARS['querystring']);

when this is parsed, isn't this essentially the same thing as using the @ from the get go? it would still end up as:

[email protected]

Edited March 7, 2008 by eww

[♥FWR Media]

if the @ symbol is harmless, i don't see the point in adding additional code to sanitizing it.

 

the problem with this is that oscommerce stores the emails with the @ in tact. so eventually it's going to need to be in there anyway if you're searching to remove a specific address.. i haven't tried to see if the ascii bits of @ would work the same way.. but if you can't get hacked by somebody passing @ in a querystring i fail to see the point in sanitizing it, especially if security pro already only allows a-z, 09 and dots and dashes.. so before and after @ are already cleansed (and are cleansed inside the unsubscribe.php codes, as well) :)

 

str_replace('-emailand-', '@', $HTTP_GET_VARS['querystring']);

when this is parsed, isn't this essentially the same thing as using the @ from the get go? it would still end up as:

[email protected]

 

The concept of Security Pro is ALLOWING characters not STRIPPING characters.

 

There is no reason for @ to be in a querystring imo.

 

[code]str_replace('-emailand-', '@', $HTTP_GET_VARS['querystring']);

when this is parsed, isn't this essentially the same thing as using the @ from the get go? it would still end up as:

[email protected][/code]

 

The end result is the same yes but it removes the need for passing unwanted characters via the querystring.

[jay2xra]

Hello,

 

I have FEC and Customer Testimonial 3.2 installed. After installing Security Pro, i can no longer receive email notifications on each order..

Any idea where to look into?

 

Thanks.

[jay2xra]

Hello,

 

I have FEC and Customer Testimonial 3.2 installed. After installing Security Pro, i can no longer receive email notifications on each order..

Any idea where to look into?

 

Thanks.

 

 

sorry, my fault.. my sendmail died... :-)

 

By the way thanks for this contrib.. I hope it will solve this claim saying if you are using FEC, your site can be hacked in less than 2 minutes

http://www.oscommerce.com/forums/index.php?sho...t=#entry1234914

[Debs]

For some time I have blocked exploits by using this in my htaccess.

This may be a silly question... but why is the contribution better then a simple htaccess like mine below?

 

Options +FollowSymLinks

RewriteEngine On

RewriteBase /

 

 

########## Begin - Rewrite rules to block out some common exploits

#

# Block out any script trying to set a mosConfig value through the URL

RewriteCond %{QUERY_STRING} mosConfig_[a-zA-Z_]{1,21}(=|\%3D) [OR]

# Block out any script trying to base64_encode crap to send via URL

RewriteCond %{QUERY_STRING} base64_encode.*\(.*\) [OR]

# Block out any script that includes a <script> tag in URL

RewriteCond %{QUERY_STRING} (\<|%3C).*script.*(\>|%3E) [NC,OR]

# Block out any script trying to set a PHP GLOBALS variable via URL

RewriteCond %{QUERY_STRING} GLOBALS(=|\[|\%[0-9A-Z]{0,2}) [OR]

# Block out any script trying to modify a _REQUEST variable via URL

RewriteCond %{QUERY_STRING} _REQUEST(=|\[|\%[0-9A-Z]{0,2})

# Send all blocked request to homepage with 403 Forbidden error!

RewriteRule ^(.*)$ index.html [F,L]

[failsafe]

Hi,

 

Great contribution. Many thanks.

 

I do have one question... in includes/functions/security.php there's the following function...

 

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

The isset line looks a rather strange version of a ternary construct and is relatively tricky to understand (esp. for newbies). Is there any reason it couldn't be written as:

 

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

or is there a special reason to write it the way it is currently? Perhaps I'm even misunderstanding what those lines actually do?

 

Ta. :)

Edited September 30, 2008 by failsafe

[♥FWR Media]

Hi,

 

Great contribution. Many thanks.

 

I do have one question... in includes/functions/security.php there's the following function...

 

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

The isset line looks a rather strange version of a ternary construct and is relatively tricky to understand (esp. for newbies). Is there any reason it couldn't be written as:

 

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

or is there a special reason to write it the way it is currently? Perhaps I'm even misunderstanding what those lines actually do?

 

Ta. :)

 

Oooh never saw this.

 

Yes it's written like that because it's the way I wrote it .. AND .. curly brackets on the same line .. YUCK!

[Flinspach]

Thank you, works fine.

 

Martin

[♥FWR Media]

Thank you, works fine.

 

Martin

 

My pleasure, hope it helps keep you safe.

[Black Jack 21]

Hi Robert,

 

one question. It's necessary to pass through special characters like äüöÄÜÖß posted from the search box. What's the way to do it? I've found it works if i modify the function like this:

 

function tep_clean_get__recursive($get_var)
 {
 if (!is_array($get_var))
 return preg_replace("/[^ {}a-zA-Z0-9ßäüöÄÜÖ_.-]/i", "", urldecode($get_var));

 // Add the preg_replace to every element.
 return array_map('tep_clean_get__recursive', $get_var);
 }

 

What do you think, do you see any problems? Please comment and point me in the right direction!

 

Thank you in advance

BJ