Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Cart modification during checkout #16

wants to merge 5 commits into


None yet
1 participant

wrwrwr commented Apr 24, 2012

Using two browser tabs a user can modify her cart after selecting a shipping/payment module, thus for example get a lower shipping rate. Steps to reproduce:

  • install a clean copy of osC 2.3 (tested with Apache/2.2.16, PHP 5.2.17 and 5.3.3);
  • enable "Per Item" shipping through admin;
  • in one tab: add something to the cart, click checkout, register, select per item shipping, continue, but don't select payment;
  • in a second tab: add something to the cart (*and visit shipping selection = click checkout);
  • in the first tab: continue to confirmation -- the item added in the second tab does not increase shipping cost, checkout does not reset.

The $cartID prevention added some time ago is ineffective due to a couple of reasons:

  • firstly, with register_globals disabled both $cartID and $cart->cartID end up pointing to the same memory -- they always match; this is due to some interplay of registering a variable before setting it and session.bug_compat_42 (which may or may not be a PHP bug), this can be demonstrated with a small example:
    // test-1.php
    // assumption: register_globals is off (.htaccess: php_value register_globals 0)
    ini_set('session.bug_compat_42', 1);

    $_SESSION['a'] = null;
    $a = 1; // $cart->cartID

    $_SESSION['b'] = null;
    $b = $a; // $cartID
    // test-2.php

    $_SESSION['a'] = 2;

    var_dump($_SESSION); // both $a and $b are equal to 2
  • secondly, the $cartID is equated to $cart->cartID on every request to checkout_shipping.php, without submitting the form -- so after adding the product to the cart, view the shipping selection page in the second tab, and continue in the first tab (with the original shipping cost; this may be used with register_globals on).
  • thirdly, the $cart->cartID is not reset in the shopping_cart->update_quantity, which is not called directly in a clean osC, but it's neither marked as private, so an uninformed developer may use it in a module or own code.

There are various ways to amend this, the code attached is my quick attempt at it -- there are some caveats:

  • unfortunately it's all done against osC 2, I've not even tried to verify the issue with 3.x;
  • only the first commit contains the actual fixes;
  • payment modules changes are completely untested;
  • all the flaws can probably be fixed in some less intrusive way.

I'd say this issue is of a lesser concern, but we've observed such a cart modification in a production store (most probably just a coincidence, rather than a willful exploit), so maybe it's worth fixing -- this way or another. Sorry for this lack of testing and the careless attitude, but maybe someone more into the system will be able to make something of it .

@haraldpdl haraldpdl added a commit that referenced this pull request Aug 7, 2014

@haraldpdl haraldpdl Merge pull request #16 from acidvertigo/patch-16
optimize tep_not_null function

@gburton gburton added a commit that referenced this pull request Aug 23, 2014

@gburton gburton Merge pull request #16 from osCommerce/master
Merge from Master

@haraldpdl haraldpdl pushed a commit that referenced this pull request Aug 23, 2014

@acidvertigo acidvertigo Merge pull request #16 from osCommerce/master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment