XSS issue (with IH4) and possibly blind SQL injection with ZC151?

The XSS issue I already reported to Divavocals too, so he/she might fix it. I can confirm the injection, but can't really exploit it myself (not being a hacker).
I haven't investigated it to the bottom, but I think the cause of the XSS issue is simple and clear if you scan the IH code for "popup_image_additional".

Since a blind SQL injection issue was reported at the same time I will send you the whole report (to you only, and with permission of the site owner).

So far I have not been able to track down the source of the blind SQL injection, but the shop is not 100% standard 151 and the issue might very well be caused by a custom modification. I will try to get a report on a default Zen Cart 151 install to see if the same issues will be found then.

The password for the document is 106699

It appears only max 56k PDF can be uploaded? Will send a link to the report later on.

Temporary link to the PDF report:

http://www.beterelektro.nl/paul/_TW__RVSgrepen___2013_06_21_20130624_38592.pdf

I've downloaded the file.

Ok, thanks. I removed it.

So far the discussion with Diva unfortunately isn't getting anywhere and seems to be nothing more than a waist of time.

Can you prepare a zip of the site's /includes/ folder, and post a link to it?

https://www.beterelektro.nl/paulus/rvs_includes_for_drbyte.zip

It's from an older date, I am quite sure there have been no file changes since. Can't make a new backup at this moment.

Link seems to not work

Sorry, new link:
http://www.beterelektro.nl/paul/rvs_includes_for_drbyte.zip

(tested this time...)

Got it. Will aim to take a look at it over the weekend.

A. From a code-inspection of the /includes/ folder files you sent, I don't see any immediately obvious injection risks.

1. But I did notice that your /includes/modules/icAcadame/create_account.php file is based on code from an old version.
   And it's also got a logic error in it: $kvk is probably supposed to be $btw here:```
   if (ACCOUNT_BTW == 'true') $kvk = zen_db_prepare_input($_POST['btw']); // added for btw- en kvk nummer

2. I also note that you've added MODULE_PAYMENT_MONEYORDER_PAYTO_EXTRA to the moneyorder.php payment module, but it's never used anywhere.

3. I see that the tell_a_friend page is still present from an older version.
3b. So is the cc.php payment module.

4. You've added Attachment support to the /includes/modules/pages/contact_us/header_php.php but haven't upgraded that file to include the anti-spam support added to v1.5.1

5. You've deleted the English language pack entirely, and replaced it with a dutch language pack.  My dutch is really weak, so I have not looked at your dutch language pack files for any validation.
Granted, SQL injections and XSS issues are rarely caused by language-pack issues.

6. You're using the iCacademe template. While a quick review of it doesn't seem to show any obvious security problems, some of the files are missing some of the improvements added to newer ZC versions. 


B. Payload testing on your site

1. Blind SQL Injection:  
a) Direct attempt: false. Can't reproduce on your site when using a browser and passing the payload request supplied. (starting with the home page, adding something to the cart to bring up the shopping_cart page with shipping-estimator present, and then attempting to hack it.
The zone_country_id is correctly sanitized, with the rogue ' marks removed thus nullifying the injection risk: [&scid=table\_table&zone\_country\_id=150%20OR%20bLIKEb](http://www.rvsgrepen.nl/index.php?main_page=shopping_cart&scid=table_table&zone_country_id=150%20OR%20bLIKEb)"

b) Browser injection attempt: **True**. 
You should see an error in your /logs/ folder from just a few minutes ago, showing a MySQL error message.
I set zone_country_id to 150'%20OR%20'b'LIKE'b in the form using Firebug (and also set scid to table_table) and submitted the form. It triggered WARNING: An error has occurred...
This means that Zen Cart encountered a database error and halted execution. 
I'd be interested in seeing the logged error message, as it will be the key to understanding it more.

Now, I can't recreate it on my development server, so it may be cause by differences in your code vs mine (I've not loaded a site using your code, because I don't have a matching database from you, and I'm not sure I want to go that far with this.)

You could try adding a temporary sanitization:
new file: /includes/functions/extra_functions/temporary_controlscan_sanitization.php ```
<?php
if (isset($_POST['zone_country_id'])) $_POST['zone_country_id'] = (int)$_POST['zone_country_id'];

2. IH4 vulnerability: True.
   The popup_image_additional tpl_main_page.php has commented-out the protection code without doing any other sanitization, thus creating the vulnerability:

//Begin Image Handler changes 1 of 2
//the next line is commented out for Image Handler 3
//  if (file_exists($_GET['products_image_large_additional'])) {
//End Image Handler changes 1 of 2
  echo '<a href="javascript:window.close()">' . zen_image($_GET['products_image_large_additional'], $products_values->fields['products_name'] . ' ' . TEXT_CLOSE_WINDOW) . '</a>';
//Begin Image Handler changes 2 of 2
//the next three lines are commented out for Image Handler 3
//  } else {
//    echo '<a href="javascript:window.close()">' . zen_image(DIR_WS_IMAGES . $products_image, $products_values->fields['products_name'] . ' ' . TEXT_CLOSE_WINDOW) . '</a>';
//  }
//End Image Handler changes 2 of 2
?

3. The other "medium"-rated issues about CSRF and forms not changing between refreshes are relatively moot, especially for the product_info page, since there's nothing sensitive going on there.

Update: the bSQLi report might be an actual core code issue, which is only triggerable when your shipping modules have a Zone Restriction associated with them.

The code change for sanitizing that I posted earlier should mitigate it until a proper fix is determined for core code.

I'd be interested not only in the logs as mentioned earlier, but also interested in knowing when you've made the code change so I can test your site again.

Cheers.

Update.

I think this is a better fix instead of adding that temporary file in extra_functions:

Find /includes/modules/shipping_estimator.php
Near the top of the file, there is the following block of code. Insert the two new lines as shown:

if (!defined('IS_ADMIN_FLAG')) {
  die('Illegal Access');
}
[B]if (isset($_POST['zone_country_id'])) $_POST['zone_country_id'] = (int)$_POST['zone_country_id'];
if (isset($_POST['scid'])) $_POST['scid'] = preg_replace('/[^a-z_0-9\-]/i', '', $_POST['scid']);
[/B]
// load JS updater

This appears to test successfully on my v151 and v160 development sites, and will probably be the official fix that we publish.

Steps to replicate:

• ensure shipping-estimator is enabled (either mode is fine, just not 0-off)
• configure at least one shipping module to have a zone-restriction
• add something to cart
• access shipping estimator (either popup or in-page)
• manipulate the DOM (firebug or tamperdata or devtools) and find the <select> for zone_country_id , then find the <option> that has selected="selected" and set its value to 150'%20OR%20'b'LIKE'b (inside "double quotes")
• submit the form

*with the fix above, you should just get Netherlands as selected country, and no error.
*Without the fix above, you will probably get an SQL 1064 error like: [select zone_id from zones_to_geo_zones where geo_zone_id = '1' and zone_country_id = '150'%20OR%20'b'LIKE'b' order by zone_id]

Hi Dr.,

thanks a lot for the extensive investigation and report! And sorry for not getting back sooner (I somehow missed the updates).

To be sure not to waist your and my time: do you still need the log or any other information from me? I guess not since you managed to replicate and solve the main issue.

Also, just for clarity: I want to mention it is not my site. It just a site I did some work on, and so the owner came back to me with the report. If needed I can access the site for the logs, but I don't have them at hand.

I need some time to absorb all the information but I will apply the suggested fix asap and try to get feedback on it. I will also pass on the information about IH to Divavocals. If he does not come up with a fix I might give it a shot myself although I rather not interfere with the IH development.

But anyway I am happy it appeared to be useful and my "gut feeling" the injection very likely was not caused by the site modifications was right,

Paul

Hi Paul,
I'm satisfied that the fix in my last post is correct.

No further logs required.

I would be interested in knowing the result of the next "scan" (after you've applied the fix) though.

We will be releasing the fix in the next few weeks after we have more time to assess whether the vulnerability can actually be exploited maliciously or not. These are always delicate issues, since explaining the fix also highlights the exact attack vector that a malicious hacker could use directly if they wanted to attack thousands of stores.

Decided to apply the blind sql fix right away, and while at it found the error log:

[06-Jul-2013 08:30:50] PHP Fatal error: 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 ''b'LIKE'b' order by zone_id' at line 1 :: select zone_id from zones_to_geo_zones where geo_zone_id = '1' and zone_country_id = '150'%20OR%20'b'LIKE'b' order by zone_id in /domains/rvsdeurgrepen.nl/public_html/includes/classes/db/mysql/query_factory.php on line 120
[06-Jul-2013 08:30:50] PHP Fatal error: Class 'queryFactoryResult' not found in /domains/rvsdeurgrepen.nl/public_html/includes/classes/db/mysql/query_factory.php on line 213

Just in case.

Yes, I asked the owner and I will forward the new report when I get it.

I have forwarded the details about the IH issue to Diva, but for the time being I have added the following quick fix for the xss issue at rvsgrepen.nl:

Modified popup_image_additional tpl_main_page.php like this:

//  if (file_exists($_GET['products_image_large_additional'])) {
// xss fix paulm
$_GET['products_image_large_additional'] = preg_replace('/[^A-Za-z0-9_\-\@.]/', '', $_GET['products_image_large_additional']);

Analogue to your scid fix, but allowing dots and _, - and @ in the filenames (I think...).

It seems to work well on rvsgrpen so far, I am not sure though which characters should be allowed (maybe only the ones that IH uses for the cache? or all characters that images names may have?).

Well , uncommenting the IF on the previous line would force it to only ever match a valid filename, thus never allowing a URL or <script> tag to be submitted, thereby preventing the problem.
Of course this all assumes the images are indeed stored locally on the same server, and not remotely at some other URL

Yes I understand, but I assumed (not checked) that the "comment out" is required for IH to work properly.