View Issue Details

Jump to Notes
IDProjectCategoryView StatusDate SubmittedLast Update
0006854OXID eShop (all versions)1.02. Price calculations (discounts, coupons, additional costs etc.)public2018-07-13 14:032021-03-17 10:14
ReporterQA Assigned To 
PrioritynormalSeveritycrashReproducibilityalways
Status resolvedResolutionfixed 
Product Version6.2.2 
Fixed in Version6.2.4 
Summary0006854: Wrong voucher calculation - discount sharing between user's baskets
DescriptionIn case of vouchers with the same number, it's possible to create a sharing-like behavior of the total discounts, which could lead to wrong basket calculation - even negative basket sums are possible!

The problem is the storing of the total discount, calculated while using a voucher with a percentage discount. The calculated total sum is stored in oxvouchers table. For any reason, it's possible that an user can use the same voucher as another user. Then he gets the pre-calculated discount, which clearly refers to another basket.

[sp]
Steps To Reproduce1) Create a voucherseries
OXDISCOUNTTYPE = percent
OXALLOWSAMESERIES = 0
OXALLOWOTHERSERIES = 0
OXALLOWUSEANOTHER = 0
OXCALCULATEONCE = 0

2) Assign a category to the voucherseries.
3) Create one voucher from the series.
4) Open browser A and login to shop with user A.
5) Add an article from the assigned category to your basket.
6) Input the voucher code.
Discount should be calculated the right way.
7) Open browser B and login to shop with user B.
8) Add another article from the assigned category to your basket.
9) Login to you shop's database.
10) Update the value oxvouchers.OXRESERVED to a timestamp older than 3 hours (in standard).
Explanation: The reservation time of the used voucher has to be run out. With standard settings, a voucher is reserved for 3 hours. So you may wait that time, but it's easier to modify the oxvouchers.OXRESERVED value. Just change it to a value thats more than 3 hours back in past, so subtract a number > 10800.
11) Finish your order.
12) Go back to browser A.
13) Update your basket.
Now you should see the total discount from user B.
Additional InformationThe scenario a little bit more technical:
1) User A inputs voucher number.
2) Voucher X gets reserved by inserting a timestamp to oxvouchers.OXRESERVED.
3) User A doesn't do anything and leaves his basket as it is for about 3 hours (in standard).
4) oxvouchers.OXRESERVED from voucher X is now more than 3 hours old, so voucher X isn't reserved anymore.
5) Now User B inputs the voucher number and since voucher X isn't reserved anymore, user B re-reserves voucher X for his basket.
6) User B finishes his order.
7) At this point the OXORDERID, OXUSERID and OXDISCOUNT were inserted into the oxvouchers table.
8) User A comes back and updates his basket.
9) Then the problem starts to begin: Even though voucher X isn't reserved anymore by user A, this voucher is used for calculation.
10) Unfortunately the calculation function read out OXDISCOUNT from the database and adds it to user A's basket.
Problem 1.1: Wrong total discount value is used for user A.
Problem 1.2: If basket sum from user A is less than discount from user B, which is stored in oxvouchers.OXDISCOUNT, basket sum could get under 0,00.
Problem 2: Same voucher X is used by two customers.
11) User A finishes his order.
12) OXORDERID and OXUSERID from voucher X is overwritten with the new values from user A.

(My guess is, that the oxvouchers.OXID from voucher X is stored in the session from user A.)
TagsCalculations, Discount, Voucher
ThemeNot defined
BrowserNot defined
PHP VersionNot defined
Database VersionNot defined

Relationships

has duplicate 0006562 closedflorian.auer Wrong Voucher Calculation - session basket mixing 

Activities

DanielaP

2020-09-02 16:52

reporter   ~0013284

Last edited: 2020-09-03 08:40

 Here is some further info because we just stumbled upon this bug in our system:
- the oxid of the voucher is saved in the oxbasket object (in the _aVouchers Array, it looks like [oxid]=>oxStd where oxStd is an object containing the voucher data, but is not of type oxvoucher)
- as soon as oxbasket->calcVoucherDiscount is called the data is overwritten (in the basket and the session)
- the bug does not occur because the oxdiscount field of the oxvoucher is loaded but because oxvoucher->_getBasketItems($oDiscount = null) first checks if oxvouchers__oxorderid is set and if so loads the oxorderarticles of this order instead of the basketitems of the current basket, this means all voucher calculations are happening on the wrong articles, so there is no exception
- this method is only called in oxvoucher->_getProductDiscoutValue($dPrice) and oxvoucher->_getCategoryDiscoutValue($dPrice), thus only vouchers with assigned categories or articles are affected

One quickfix to at least make this a much rarer occurence would be to order the sql query that gets a new voucher by oxtimestamp (in oxvoucher->getVoucherByNr()), that way the next voucher drawn would be guaranteed to have been untouched for the longest time (oxtimestamp is updated on reserving, too)
I don't know how that would affect performance though. But right now on our system the next voucher you get will be the one one oxid up alphabetically from the last, which means as soon as it is free it will be reassigned

A possible solution to the problem would be adding the folling into oxbasket->_calcVoucherDiscount(), directly above the call of $oVoucher->getDiscountValue($dPrice), but I am unsure if this will affect anything else. I assume checking the oxorderid will not work with various recalculations, but there should be no vouchers that can be used after they were already used by someone else, right?

                            if(($oVoucher->oxvouchers__oxdateused->value!=='000-00-00') && $oVoucher->oxvouchers__oxuserid->value && ($oVoucher->oxvouchers__oxuserid->value!==$this->getBasketUser()->getId())) {
                                $oEx = oxNew('oxVoucherException');
                                $oEx->setMessage('MESSAGE_COUPON_EXPIRED');
                                $oEx->setVoucherNr($oVoucher->oxvouchers__oxvouchernr->value);
                                throw $oEx;
                            }

Another idea would be to keep the reserved time in the _aVoucher Array, too and check this again whenever the voucher discount is calculated. However that would mean a lot more changes

QA

2020-09-04 13:00

administrator   ~0013287

also reproducable in OXID eShop V6.2.2

-es-