Jump to content

Archived

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

longhorn1999

Addendum to Security Pro

Recommended Posts

Hi everyone,

 

In the popular 'How to secure your site' thread by Spooks, he mentions putting this bit of code into certain pages:

 

 

FORMS:

 

Security Pro cleans the query string, however any forms using $_POST are un-affected, if you have any forms using the post method you would be advised to do the following on pages accepting $_POST vars.

 

after:

 

require('includes/application_top.php');

 

 

add:

 

 

// clean posted vars

reset($_POST);

while (list($key, $value) = each($_POST)) {

if (!is_array($_POST[$key])) {

$_POST[$key] = preg_replace("/[^ a-zA-Z0-9@%:{}_.-]/i", "", urldecode($_POST[$key]));

} else { unset($_POST[$key]); } // no arrays expected

}

 

 

 

This does not allow for arrays, additional code is needed if they are used.

 

I'm wondering if this code would help me with 2 PCI errors McAfee keeps showing in my scans:

 

MySQL Database Error Disclosure Vulnerability, Potentially Exploitable Database Error Message

 

 

McAfee tells me that it's a legitimate error since if I enter http://www.my-domain.com/?cPath=21&sort=2a&max=x' it'll output a 1064 error and a long message.

 

Security Pro has worked great for me, but I can't figure out these errors and so I'm hoping this extra code may help. For non-coders like myself, is there a short list of files that should have this modification (create_account, contact_us, etc)? Or should I try to install Spook's anti-hacker account mods, which looks like it'll be very difficult to set up properly?

 

Thanks for the help,

 

Nick

Share this post


Link to post
Share on other sites

Hi everyone,

 

In the popular 'How to secure your site' thread by Spooks, he mentions putting this bit of code into certain pages:

 

 

 

 

I'm wondering if this code would help me with 2 PCI errors McAfee keeps showing in my scans:

 

MySQL Database Error Disclosure Vulnerability, Potentially Exploitable Database Error Message

 

 

McAfee tells me that it's a legitimate error since if I enter http://www.my-domain.com/?cPath=21&sort=2a&max=x' it'll output a 1064 error and a long message.

 

Security Pro has worked great for me, but I can't figure out these errors and so I'm hoping this extra code may help. For non-coders like myself, is there a short list of files that should have this modification (create_account, contact_us, etc)? Or should I try to install Spook's anti-hacker account mods, which looks like it'll be very difficult to set up properly?

 

Thanks for the help,

 

Nick

 

It has nothing to do with cleansing, that particular querystring causes your site to emit an error message .. error messages should not be displayed to screen but also that code should not cause an error in the first place. It is a case of finding the code that is responding to that querystring and correcting it.

Share this post


Link to post
Share on other sites

It has nothing to do with cleansing, that particular querystring causes your site to emit an error message .. error messages should not be displayed to screen but also that code should not cause an error in the first place. It is a case of finding the code that is responding to that querystring and correcting it.

 

 

Thanks Robert. For the benefit of myself and other newbies, how would I go about finding the offending code, since I'm not sure at all what files/contributions are causing this issue? Do I simply disable or delete certain add-ons or pages temporarily and use a trial and error method? Or are there any specific forms or contributions which are known to have this database error message issue?

Share this post


Link to post
Share on other sites

Thanks Robert. For the benefit of myself and other newbies, how would I go about finding the offending code, since I'm not sure at all what files/contributions are causing this issue? Do I simply disable or delete certain add-ons or pages temporarily and use a trial and error method? Or are there any specific forms or contributions which are known to have this database error message issue?

 

1064 is a MySQL syntax error, the error would also have supplied some detail ( without which we are all blind ).

 

The _GET key "max" is not however standard in osCommerce.

Share this post


Link to post
Share on other sites

1064 - You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '-0, x' at line 1

select p.products_image, pd.products_name, p.products_quantity, p.products_id, pd.short_desc, p.manufacturers_id, p.products_price, p.products_tax_class_id, p.products_status, IF(s.status, s.specials_new_products_price, NULL) as specials_new_products_price, IF(s.status, s.specials_new_products_price, p.products_price) as final_price from products_description pd, products p left join manufacturers m on p.manufacturers_id = m.manufacturers_id left join specials s on p.products_id = s.products_id, products_to_categories p2c where p.products_id = p2c.products_id and pd.products_id = p2c.products_id and pd.language_id = '1' and p2c.categories_id = '21' order by pd.products_name limit -0, x

 

This is the specific error message I get with http://www.my-domain.com/?cPath=21&sort=2a&max=x'. It also shows results pages in this category of -4,-3,-2,-1,0 instead of 1,2,3... My product_info.php is heavily modified with Spooks' Product Listing Enhancements and other things. Would you know if it is actually some database error or is it something in the code that needs to be edited?

 

Thanks again Robert

Share this post


Link to post
Share on other sites

1064 - You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '-0, x' at line 1

select p.products_image, pd.products_name, p.products_quantity, p.products_id, pd.short_desc, p.manufacturers_id, p.products_price, p.products_tax_class_id, p.products_status, IF(s.status, s.specials_new_products_price, NULL) as specials_new_products_price, IF(s.status, s.specials_new_products_price, p.products_price) as final_price from products_description pd, products p left join manufacturers m on p.manufacturers_id = m.manufacturers_id left join specials s on p.products_id = s.products_id, products_to_categories p2c where p.products_id = p2c.products_id and pd.products_id = p2c.products_id and pd.language_id = '1' and p2c.categories_id = '21' order by pd.products_name limit -0, x

 

This is the specific error message I get with http://www.my-domain.com/?cPath=21&sort=2a&max=x'. It also shows results pages in this category of -4,-3,-2,-1,0 instead of 1,2,3... My product_info.php is heavily modified with Spooks' Product Listing Enhancements and other things. Would you know if it is actually some database error or is it something in the code that needs to be edited?

 

Thanks again Robert

 

And there's the issue .. limit -0, x

 

LIMIT requires an integer .. x of course is not an integer therefore the error.

 

Badly written insecure code. The value of $_GET['max'] is being directly injected into the query whereas it should have been type cast to (int).

Share this post


Link to post
Share on other sites

Ok, great info Robert. Let's see if I can cobble together a fix for this and actually pass the McAfee scans without calling everything a false positive! :)

 

Am I correct in assuming that the problem lies somewhere in my product_info.php or could it be an issue with the thumbnails contribution(osCthumb), as referenced in this other thread?

 

http://forums.oscommerce.com/topic/350854-how-to-make-osc-pci-compliance-and-pass-mcafee-secure-scan/page__p__1467642__hl__robots__fromsearch__1&

Share this post


Link to post
Share on other sites

Ok, great info Robert. Let's see if I can cobble together a fix for this and actually pass the McAfee scans without calling everything a false positive! :)

 

Am I correct in assuming that the problem lies somewhere in my product_info.php or could it be an issue with the thumbnails contribution(osCthumb), as referenced in this other thread?

 

No the url you posted was http://www.my-domain.com/?cPath=21&sort=2a&max=x'

 

There is no file there so it must be loading index.php .. so .. the "max" is either in index.php or includes/modules/product_listing.php

Share this post


Link to post
Share on other sites

Thanks once again Robert. I'm sure others will find this useful as I've seen this problem elsewhere in other threads without any definitive fix.

Share this post


Link to post
Share on other sites

1064 - You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '-0, x' at line 1

select p.products_image, pd.products_name, p.products_quantity, p.products_id, pd.short_desc, p.manufacturers_id, p.products_price, p.products_tax_class_id, p.products_status, IF(s.status, s.specials_new_products_price, NULL) as specials_new_products_price, IF(s.status, s.specials_new_products_price, p.products_price) as final_price from products_description pd, products p left join manufacturers m on p.manufacturers_id = m.manufacturers_id left join specials s on p.products_id = s.products_id, products_to_categories p2c where p.products_id = p2c.products_id and pd.products_id = p2c.products_id and pd.language_id = '1' and p2c.categories_id = '21' order by pd.products_name limit -0, x

 

This is the specific error message I get with http://www.my-domain.com/?cPath=21&sort=2a&max=x'. It also shows results pages in this category of -4,-3,-2,-1,0 instead of 1,2,3... My product_info.php is heavily modified with Spooks' Product Listing Enhancements and other things. Would you know if it is actually some database error or is it something in the code that needs to be edited?

 

Thanks again Robert

 

It is Product Listing Enhancements problem. I am using Macfee secure too, after I remove Product Listing Enhancements from my system, it pass the scan. If you pass it without remove Product Listing Enhancements, please let me know how, I really like to find out.

ken

Share this post


Link to post
Share on other sites

Hi

 

The problem is in Sams Product Listing Enhancements. \includes\modules\product_listing.php around line 24.

 

$max_results = (tep_not_null($_GET['max']) ? $_GET['max'] : $max_rows);

 

But there is nothing in Sams code that could cause $_GET['max'] to equal ‘x’

 

$_GET['max'] should be the value of the results/page drop down box.

 

The ‘x’ must have been inserted into the URL by McAfee.

 

 

This is what I did to prevent the problem

 

Change

 

$max_results = (tep_not_null($_GET['max']) ? $_GET['max'] : $max_rows);

 

To

 

$max_results = (isset($_GET['max']) ? intval($_GET['max']) : MAX_DISPLAY_SEARCH_RESULTS);
 if ($max_results == 0 ){$max_results = MAX_DISPLAY_SEARCH_RESULTS;}

 

The ‘intval’ sanistises the string and changes ‘x’ to 0 however a 0 injected into the query will still cause an SQL error.

The second line changes the 0 to MAX_DISPLAY_SEARCH_RESULTS

 

The code appears to work although I am sure could be better written.

 

Hope this helps

 

Ken.

Share this post


Link to post
Share on other sites

Hi

 

The problem is in Sams Product Listing Enhancements. \includes\modules\product_listing.php around line 24.

 

$max_results = (tep_not_null($_GET['max']) ? $_GET['max'] : $max_rows);

 

But there is nothing in Sams code that could cause $_GET['max'] to equal ‘x’

 

$_GET['max'] should be the value of the results/page drop down box.

 

The ‘x’ must have been inserted into the URL by McAfee.

 

 

This is what I did to prevent the problem

 

Change

 

$max_results = (tep_not_null($_GET['max']) ? $_GET['max'] : $max_rows);

 

To

 

$max_results = (isset($_GET['max']) ? intval($_GET['max']) : MAX_DISPLAY_SEARCH_RESULTS);
 if ($max_results == 0 ){$max_results = MAX_DISPLAY_SEARCH_RESULTS;}

 

The ‘intval’ sanistises the string and changes ‘x’ to 0 however a 0 injected into the query will still cause an SQL error.

The second line changes the 0 to MAX_DISPLAY_SEARCH_RESULTS

 

The code appears to work although I am sure could be better written.

 

Hope this helps

 

Ken.

 

 

Thanks Ken! Your code seems to have fixed the problem for me as well.

Share this post


Link to post
Share on other sites

×