Question about replacing [Get]s in addon code for 1.5

[Diavire]

I see, misunderstood the context then.

In my reasoning the "not possible" referred to disabling the possibility to simply create an url with gets (by adding or changing entirely to a post), making the overall exploit more difficult, but still possibly (using a server to server approach like cURL).

Since I misunderstood the context, the "not possible" and "impossible" thing don't make any sense whatsoever

Sometimes you don't learn until you've asked some stupid questions, so thanks for explaining. :)

[dbltoe]

SO.... If my foggy brain is absorbing this, the following code only needs the $_GET on several lines changed to $_POST to meet standards. Yes or no?

<?php
/**
 * Module Template
 *
 * @package templateSystem
 * @copyright Copyright 2007 FUAL
 * @copyright Portions 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_modules_main_product_image.php 2007-12-04 btyler $
 */
?>
<?php require(DIR_WS_MODULES . zen_get_module_directory(FILENAME_MAIN_PRODUCT_IMAGE)); ?> 
<div id="productMainImage" class="centeredContent back">
<!-- bof Zen Slimbox v0.1 btyler 2007-12-04 -->
<?php
if( FUAL_SLIMBOX == 'true' || ZEN_LIGHTBOX_STATUS == 'true' ) {
    // Set the title
    if ( $current_page_base == 'product_reviews' ) {
        $fual_slimbox_title = htmlentities($review->fields['products_name'],ENT_QUOTES);
    } else {
        $fual_slimbox_title = htmlentities($products_name,ENT_QUOTES);
    }
    // Get the href for the large image
    $fual_slimbox_href = zen_lightbox($products_image_large, $fual_slimbox_title, LARGE_IMAGE_WIDTH, LARGE_IMAGE_HEIGHT);
    $fual_slimbox_a = '<a href="' . $fual_slimbox_href . '" rel="lightbox[gallery]" title="' . $fual_slimbox_title . '">';
    // Get the img element for this product.
    $fual_slimbox_image = zen_image($products_image_medium, $fual_slimbox_title, MEDIUM_IMAGE_WIDTH, MEDIUM_IMAGE_HEIGHT); 

    // Test remote images to simulate slow loading for local development
    //$fual_slimbox_image = '<img src="http://demos.mootools.net/demos/DomReadyVS.Load/moo.png" width="150px" height="150px;" alt="test" />'; 
    
    // Note if you want to test a slow DOM load, then in /index.php add sleep(5); (in php brackets) just before the final </html>
    // This will make the DOM take an extra 5 seconds to load, which simulates dialup (what a cool feature)
    
    $fualSlimboxContent = "";
    if( ZEN_LIGHTBOX_STATUS == 'true' ) {
        $fualNervousSwitch = 0;
    } else {
        $fualNervousSwitch = FUAL_SLIMBOX_NERVOUS;
    }
    switch( $fualNervousSwitch ) {
        case 2:
            $fualSlimboxContent .= '<div id="slimboxWrapper">';
            break;
        case 1:
            $fualSlimboxContent .=  '<div id="slimboxWrapper" style="display:block;">';
            break;
        case 0:
        default:
            $fualSlimboxContent .= '<div id="slimboxWrapper" style="display:block; visibility:visible;">';
    }
    $fualSlimboxContent .=  $fual_slimbox_a . $fual_slimbox_image . '</a>'; 
    // Putting the text link together with the image is nasty!
    $fualSlimboxContent .=  '<br class="clearBoth" />';
    $fualSlimboxContent .=  $fual_slimbox_a . '<span class="imgLink">' . TEXT_CLICK_TO_ENLARGE . '</span></a>';
    $fualSlimboxContent .=  '</div>';
?>
    <script language="javascript" type="text/javascript"><!--
    document.write('<?php echo $fualSlimboxContent; ?>' );
    //--></script>
    <noscript>
    <?php
    // If they can't be bothered to get a decent browser or turn js on then they only deserve the default behaviour.
    echo '<a href="' . zen_href_link(FILENAME_POPUP_IMAGE, 'pID=' . $_GET['products_id']) . '" target="_blank">' . zen_image($products_image_medium, $products_name, MEDIUM_IMAGE_WIDTH, MEDIUM_IMAGE_HEIGHT) . '<br /><span class="imgLink">' . TEXT_CLICK_TO_ENLARGE . '</span></a>';
?>
    </noscript>
<?php         
} else {
?>
<!-- bof Zen Slimbox v0.1 btyler 2007-12-04 -->
    <script language="javascript" type="text/javascript"><!--
    document.write('<?php echo '<a href="javascript:popupWindow(\\\'' . zen_href_link(FILENAME_POPUP_IMAGE, 'pID=' . $_GET['products_id']) . '\\\')">' . zen_image($products_image_medium, addslashes($products_name), MEDIUM_IMAGE_WIDTH, MEDIUM_IMAGE_HEIGHT) . '<br /><span class="imgLink">' . TEXT_CLICK_TO_ENLARGE . '</span></a>'; ?>');
    //--></script>
    <noscript>
    <?php
    echo '<a href="' . zen_href_link(FILENAME_POPUP_IMAGE, 'pID=' . $_GET['products_id']) . '" target="_blank">' . zen_image($products_image_medium, $products_name, MEDIUM_IMAGE_WIDTH, MEDIUM_IMAGE_HEIGHT) . '<br /><span class="imgLink">' . TEXT_CLICK_TO_ENLARGE . '</span></a>';
    ?>
    </noscript>
<?php } ?>
</div>

[kuroi]

SO.... If my foggy brain is absorbing this, the following code only needs the $_GET on several lines changed to $_POST to meet standards. Yes or no?

No.

I doubt that any changes are needed. They rarely (maybe never) will be on the store side of a site unless they relate to the checkout or the account area, as these are the only places where database changes are likely to occur.

I recommend looking back specifically at post #2 earlier in this thread for an explanation of the limited circumstances when $_GET variables could create security vulnerabilities.

[dbltoe]

THANX
Still trying to wrap my head around this.
Know that it works with 1.5 without the "monitors" shutting it down a la IH3.
Nest stop.... The 37 $_GET calls in ih_manager.php

[delia]

I just have to have some more specifics about this! Just not feeling confident of my understanding.

For example, this line: $action = (isset($_GET['action']) ? $_GET['action'] : ''); has nothing to do with the actual database changes - it's just picking up what the action is. Same for this: switch($_GET['action'])

This one sets the form action as get and not post: <?php echo zen_draw_form('clean_cross', FILENAME_CROSS_SELL_PRODUCTS, 'action=select_cross_sell', 'get'); ?> But it looks like it's just trying to choose which table to work on and works no changes on the database

This one uses post so is not a problem <?php echo zen_draw_form('clean_cross', FILENAME_CROSS_SELL_PRODUCTS, 'action=cleancross_sell', 'post'); ?> Looks like all of the actions that make database changes are done that way.

This one changes the database but is not part of a form per se though must be the result of that choice of table mentioned before:

if(defined('CROSS_SELL_ENABLED') ) {

if (isset($_GET['select_cross_sell'])) {
$cross_sell_edit = ($_GET['select_cross_sell']);
$db->Execute("UPDATE " . TABLE_CONFIGURATION .
" set configuration_value = $cross_sell_edit
WHERE configuration_key = 'CROSS_SELL_SELECTED_TABLE'" );
zen_redirect(zen_href_link(FILENAME_CROSS_SELL_PRODUCTS));
}

So my conclusion is that no changes are necessary. Does that sound right?

[potteryhouse]

Hello,
I must ask for a little help.

I am trying to update a sales tax mod to protect against the $_get vulnerability, but am not quite able to find the correct change for the section of code listed below. Any guidance / help would be appreciated. I have this mod working (locally on a test machine) with Zen Cart 1.5, but wanted to update this section before posting the changes.

The sample below is but one of four pieces that do insert, save, update, and delete. All have the same format, so once one of them are updated the others should be easy.
In-case your interested in the mod I am looking at updating, this is the link.
http://www.zen-cart.com/downloads.php?do=file&id=422 (Local Sales Tax Mod)

First is a question, does this code even need to be updated, I believe it does based on what I have read in the forum...
Second, if it does, what changes will make this work (I understand I will have update the post back from get to post etc. when this is updated).

      $heading[] = array('text' => '<b>' . TEXT_INFO_HEADING_NEW_LOCAL_SALES_TAX . '</b>');

      $contents = array('form' => zen_draw_form('local_sales_tax', FILENAME_LOCAL_SALES_TAXES, 'page=' . $_GET['page'] . '&action=insert'));
      $contents[] = array('text' => TEXT_INFO_INSERT_INTRO);
      $contents[] = array('text' => '<br>' . TEXT_INFO_COUNTRY . '<br>' . zen_draw_pull_down_menu('zone_country_id', zen_get_countries(TEXT_ALL_COUNTRIES), '', 'onChange="update_zone(this.form);"'));
      $contents[] = array('text' => '<br>' . TEXT_INFO_COUNTRY_ZONE . '<br>' . zen_draw_pull_down_menu('zone_id', zen_prepare_country_zones_pull_down()));
      $contents[] = array('text' => '<br>' . TEXT_INFO_TAX_RATE . '<br>' . zen_draw_input_field('tax_rate'));
      $contents[] = array('text' => '<br>' . TEXT_INFO_FIELDMATCH . '<br>' . zen_draw_pull_down_menu('tax_fieldmatch', $za_lookup));
            
      $contents[] = array('text' => '<br>' . TEXT_INFO_DATAMATCH . '<br>' . zen_draw_textarea_field('tax_datamatch', false, 35, 4));
      $contents[] = array('text' => '<br>' . TEXT_INFO_RATE_DESCRIPTION . '<br>' . zen_draw_input_field('tax_description'));
            
      $contents[] = array('text' => '<br />' . TEXT_INFO_TAX_SHIPPING . '<br />' . zen_draw_radio_field('tax_shipping', 'false', true) . ' ' . TEXT_TAX_SHIPPING_FALSE . '<br />' . zen_draw_radio_field('tax_shipping', 'true') . ' ' . TEXT_TAX_SHIPPING_TRUE);
      $contents[] = array('text' => '<br>' . TEXT_INFO_TAX_CLASS_TITLE . '<br>' . zen_tax_classes_pull_down('name="tax_class_id" style="font-size:10px"'));
      $contents[] = array('align' => 'center', 'text' => '<br>' . zen_image_submit('button_insert.gif', IMAGE_INSERT) . '&nbsp;<a href="' . zen_href_link(FILENAME_LOCAL_SALES_TAXES, 'page=' . $_GET['page']) . '">' . zen_image_button('button_cancel.gif', IMAGE_CANCEL) . '</a>');


//As I see it the following line needs to be updated.
$contents = array('form' => zen_draw_form('local_sales_tax', FILENAME_LOCAL_SALES_TAXES, 'page=' . $_GET['page'] . '&action=insert'));

//And, this one might need to be updated.
 $contents[] = array('align' => 'center', 'text' => '<br>' . zen_image_submit('button_insert.gif', IMAGE_INSERT) . '&nbsp;<a href="' . zen_href_link(FILENAME_LOCAL_SALES_TAXES, 'page=' . $_GET['page']) . '">' . zen_image_button('button_cancel.gif', IMAGE_CANCEL) . '</a>'); 

Thanks in advance for any suggestions / help.

Brent