Sensei
- Join Date:
- Jan 2004
- Posts:
- 63,513
- Plugin Contributions:
- 177
Image Handler 4 (for v1.5.x) Support Thread
<?php /**mod Image Handler 4.3.2 * Override Template for common/tpl_main_page.php * * @package templateSystem * @copyright Copyright 2005-2006 Tim Kroeger * @copyright Copyright 2003-2005 Zen Cart Development Team * @copyright Portions Copyright 2003 osCommerce * @license http://www.zen-cart.com/license/2_0.txt GNU Public License V2.0 * @version $Id: tpl_main_page.php,v 2.0 Rev 8 2010-05-31 23:46:5 DerManoMann Exp $ */ ?> <body id="popupAdditionalImage" class="centeredContent" onload="resize();"> <div> <?php echo '<a href="javascript:window.close()">' . zen_image(strip_tags($_GET['products_image_large_additional']), $products_values->fields['products_name'] . ' ' . TEXT_CLOSE_WINDOW) . '</a>'; ?> </div> </body> ``` > DO NOT USE THIS CODE UNTIL IT IS VERIFIED AS SAFE!!! > > That said here's an alternate.. > > ``` <?php /**mod Image Handler 4.1 * Override Template for common/tpl_main_page.php * * @package templateSystem * @copyright Copyright 2005-2006 Tim Kroeger * @copyright Copyright 2003-2005 Zen Cart Development Team * @copyright Portions Copyright 2003 osCommerce * @license http://www.zen-cart.com/license/2_0.txt GNU Public License V2.0 * @version $Id: tpl_main_page.php,v 2.0 Rev 8 2010-05-31 23:46:5 DerManoMann Exp $ */ ?> <body id="popupAdditionalImage" class="centeredContent" onload="resize();"> <div> <?php echo '<a href="javascript:window.close()">' . zen_image(htmlspecialchars($_GET['products_image_large_additional']), $products_values->fields['products_name'] . ' ' . TEXT_CLOSE_WINDOW) . '</a>' ?> </div> </body> ``` While either of those approaches will prevent the abuse of rogue HTML characters from causing trouble (XSS), it doesn't prevent the risks of specifying a path to a file that exists outside expected image folder locations (CSRF). ie: one could just manually change the image filename on the URL from images/large/IMG123.jpg to includes/templates/template_default/images/down_for_maintenance.gif ... or something dangerous like ../../../../../../etc/passwords to attempt to access files entirely outside of the website and into the core operating system. That said, attempts to access ../../../etc/passwords would fail to disclose the actual file contents to the browser because webservers won't output the contents of those files via an IMG tag.DivaVocals:
DivaVocals:
Dunno if this is CORRECT or resolves the XSS issues that was the inciting reason why IH4 was updated to begin with.. This code works, but I need someone to validate if it resolves the XSS issue that was identified in IH4 v4.1.
But you could put a URL in there and use it to load a malicious image file from a COMPLETELY DIFFERENT SERVER, and have it run the rogue code in your browser, and thus infect your computer, taint your browser cache, start rogue sessions someplace, steal your cookie data, and trick you into disclosing admin or other password credentials.
Consider this:
http://example.com/index.php?main_page=popup_image_additional&pID=123&pic=0&products_image_large_additional=http://www.zen-cart.net/images/test_demo.jpg
While that produces an image of 0 height or 0 width because the file can't be located on the local server, it does still set the src= value to the external URL because IH isn't verifying that the file exists on the local server, and is just allowing any random file to be loaded into that IMG tag, regardless of whether that file is malicious or not.
That's why the original ZC code at least runs file_exists() on $_GET['products_image_large_additional'] to make sure the file is present locally.
I'd suggest that if the previous code "change" is causing problems with finding the correct image and thus triggering the NoPicture response, that something's wrong with what's being passed as the image URL when the popup link is generated. So that's in the IH additional_images module or the template, not in the popup code which is where you were looking in the code questions you posted about today.
