Community Add-Ons

Full Package

Changelog:

Variables added to $GLOBALS via the querystring escaped our attention in the last version, this meant that $GLOBALS like $testimonials_id were not being cleansed. In this version we alter the $GLOBALS variable to the $_GET cleansed version.

This obviously is going to cause a few issues. One of which was the payment system PROTX. The way we have got around this is to add a setting in admin where you can EXCLUDE specific "known to be safe" files from the cleansing process. In the case of PROTX it is e.g. protx_process.php. So if you test well and find any files that are not functioning due to our added security you can now either adjust those files to meet the requirements of FWR Security Pro, or you can exclude those files from the cleansing process if you feel they are safe.


Included is a tougher catalog/includes/functions/security.php

The changes are minimal but it makes a big difference. The rest of the package remains the same.

Changes:
% character removed
$get_var is urldecoded before the preg_replace strips bad characters.


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 possible)

2) Exclude the file from cleansing only if you really have to. (Should only be critical operations like payment)

3) NEVER alter the preg_replace or other functions in security.php unless improving it (making it tougher).
In the case of this contribution "Yaaay I got it to work by removing X in security.php!" = you broke your security.

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). It is my belief that e.g. information pages has similar issues amongst who knows how many other contributions.

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.

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 e.g. payment systems) - Thanks perfectpassion.


We are zealously cleaning here so there is always a risk that some contribution 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.

Hope it keeps you all safe.

License: Refer to standard osCommerce license.