[TYPO3-commerce] *SECURITY ISSUE* possible Hack of paypal2ogone extension

[Thibaut van de Mortel]

Hello Martin,
hello Ingo,
hello list !
thank you for your replies. I haven't received the patch yet, so I 
decided to have a look at pi3 to see what I could do.
I know it's bad to alter Commerce's code, but I found a pretty handy 
solution with "low altering". I keep track of my changes, so I can in 
worst case repeat them after a Commerce's update if the patch doesn't 
fit my needs.

I'm not sure about this but the session data can be lost isn't it? If 
the customer logs out and then logs in with a different account for 
example ; isn't all his data lost? hmmm... obviously not as I can see 
that my basket is still the same when I log in with a different account. 
I'm sorry about those newbie considerations, but as I'm not familiar 
with the session, and as this session seems to be 1) editable 2)used by 
Commerce to insert the order in the database AFTER payment confirmation 
; then it seems to me that any payment procedure outside merchant's site 
is potentially dangerous.

I don't know, can we really trust session data when payment is made 
outside merchant's site?

As I'm not sure, I don't trust it. That's why I slightly modified 
tx_commerce_pi3.php on my installation. What I did is this :

1) cut a continuous block of code inside the function "finishIt". This 
block of code is the minimum required to insert the order to database.
2) paste it into a new function called "insertOrderToDB" (I also had to 
redeclare $hookObjectsArr and $basket within this function).
3) And of course, in the place of where I cut the code from, I insert a 
conditional call to that function :

if(!$paymentObj->hasInsertedOrderToDB()){
	$this->insertOrderToDB($paymentObj);
}

I precise that those changes don't affect in any case the process of 
other payment methods, it only disables the insert of the order in the 
database during "final finishing" (sorry but the finishIt function of 
pi3 has several steps hehe) IF the payment method returns true to the 
call; which it would do if it has already inserted the order into 
database for example hehe.

In my case, as I don't trust the session data, my payment method is 
calling the "insertOrderToDB" function via the parentObj (already 
provided by pi3) during the "getSpecialFinishingForm" function. That 
function -still in my case- also displays a form with a series of 
required Ogone hidden fields. That form will be submitted to Ogone by 
the customer ; then the customer leaves merchant's site to Ogone's.

That way, the order data is inserted in the database just before the 
customer leaves merchant's site (without being yet handled by stock 
handling however). Now the customer may do crazy things with his session 
data in another browser window, it doesn't matter. If the merchant's 
site now receives a confirmation from the payment service, there should 
be no problem to handle it, whatever happened to session meanwhile. 
Commerce will continue the finishing procedure when customer comes back, 
it will just not insert a second record in the database as my payment 
method will tell it not to. The finishing procedure of Commerce will 
automatically handle stock too.

Now the problem is : what if the customer doesn't go to the end of 
payment procedure? Well, it results of a useless record in the database, 
with a payment_ref_id set to "WAITING" or whatever string I'll choose.

[Sorry I forgot to say that I also added in the function 
"insertOrderToDB" the possibility to merge current $orderData from pi3 
with $orderData from $paymentObj if it is set and contains something]

To prevent the abuse of such useless records, I automatically delete any 
unconfirmed previous orders (*) from the current user each time he 
creates a new one.
(*) those orders will have to be "old" otherwise the customer could 
delete an order record by playing in a second window but still confirm 
the payment! I guess I should eleminate all unconfirmed order that are 
older than 1 or 2 hours.
However, the problem will still remain. Some crazy user can easily flood 
the database with the game of "back - submit - back - submit - back 
...". So it may be safe to limit the number of "allowed unconfirmed 
orders per user" to... let's say 50.
So if the "customer" (who now begins to look like a hacker doesn't he?) 
manages to erase the initial database record (after having created 50 
unconfirmed orders within the 2 hours) and still confirms the payment of 
the initial order ; then we are lost, he won. We have a payment and we 
dont know how to handle it because we lost data record and session data 
is obsolete.
I know this sounds very paranoid >_< ... in such extreme cases I guess 
we offer to that customer a coupon of the same amount of what the 
payment service notification says... if it is legal... just to avoid 
refunding him hehe >_<
I guess I'll still provide a button in the backend that automatically 
deletes any unconfirmed order that is more than 1 or 2 hour old.

I know that the cleaning process isn't perfect, that's why I think I may 
create a new order folder called "unconfirmed" or something like that. 
That way the merchant would not be flooded in his "incoming orders" 
backend folder. Of course when the order receives confirmation, the 
update process moves it to the regular pid of "incoming" order folder.

Sorry this message is becoming very long ; all that to say : am I 
paranoid with session data or can we really trust it in the case of a 
payment outside merchant's site?

If we cannot really trust session data, then it would be great that 
Commerce offers to a payment method the possibility to call a function 
that inserts the order in the database, and also the possibility to 
merge the data with some other data provided also by the payment method.

That way I can even submit my extension on TER, it will work with 
regular Commerce installation. This extension can be usefull for 
belgians developpers, because I think that Ogone is the only payment 
service that proposes all main belgian banks payment methods, plus the 
regular American Express, Visa etc...

Please let me know when the patch will be released or if by chance you 
want more info about the slight changes I made to pi3 in my installation 
and/or the Ogone thing.

Thank you for your support.

Regards,
Thibaut