osCommerce/oscommerce2 — pull request #16 by wrwrwr (2012-04-24 18:18:33+00:00)
Title: Cart modification during checkout

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 the 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:

```php
cartID

$_SESSION['b'] = null;
$b = $a; // $cartID
?>
```

```php

```

* 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 the code is 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 that'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 .