Great work by the way. It would be nice to modify the original on my site if that's ok to reflect these good modifications? I haven't had time to work on these myself for a while, so it's good to see that the community is still actively working on it. Full credit will be given, of course :)
From 1240928160:
No security hole in the module given by PrestaShop.
Wow now there's a bold claim :lol: I'm sure you didn't really mean to say that ;)
In validation.php the post variables are those returned by Paypal, and I don't personally see that they're a risk, given that they are generated by ourselves, posted by us to PayPal, returned by PayPal and then verified that they haven't been tampered with between PayPal and ourselves (via the IPN callback to PayPal in validation.php).
If PayPal returns invalid data, then there's an error in the transaction, so it could easily be viewed that forcing the type to an integer/string is actually bad practice, and better you would validate the values returned from PayPal to ensure that they are the correct type (e.g. numeric, non-zero) - if you really feel it's necessary at all - which it isn't in my opinion. The IPN mechanism is very hard to manipulate, unless you can modify the script as part of the exploit - in which case your intval() and strval() calls won't help you!
One of the problems with adding those intval() and strval() calls (certainly in the validation.php script) is that you're actually manipulating the data returned by PayPal and should there be a problem, you may well have lost valuable diagnostic data. I would certainly remove them from the debug entries, and log the actual POST variables at the very least.
Where this appears to still be open to manipulation is modification of the form posted to PayPal FROM the store, for example by changing the amount to pay (paypal.tpl) - the IPN return values should really be checked by the validation.php script to detect such manipulation. This isn;t as trivial as it seems, given that you potentially have to deal with currency conversions too.
As I said, good work and I'll have a look at the diffs to see what's changed and maybe see if there's anything else that could be added to make this even better :)
Paul