search

monero-integrations / monerowp

Monero WooCommerce Plugin for Wordpress
MIT License
105 stars 74 forks source link

Critical exploit: Cookie used to set XMR exchange rate #11

Closed evantbyrne closed 7 years ago

evantbyrne commented 7 years ago

The payment gateway class stores the exchange rate in $_COOKIE['rate']. Cookies can be modified by the HTTP client (web browser). This effectively allows customers to modify prices. To fix, remove the rate cookie altogether and store the exchange rate in the database or another server-side cache.

Source of exploit: https://github.com/monero-integrations/monerowp/blob/5d043b0150e1511e858f6825092161ec45fc099c/monero/include/monero_payments.php#L166

Cheers,

– Evan

cryptochangements34 commented 7 years ago

First off, thanks for expressing your concern! Yes, this was a very obvious concern when deciding to store the exchange rate client side, as a result line 168 if($xmr_live_price - $rate_cookie >= 1 || $xmr_live_price - $rate_cookie >= -1) compares the actual live exchange rate against the exchange rate in the stored cookie so that if the difference changes by more than 1 fiat denomination (1 Euro, 1 Dollar, etc), either by market forces or by cookie editing, then the latest exchange rate will be used. Although this seems likea good solution to me, i am no security expert. I tested this solution against basic cookie editing, but if you can still manage to exploit this, please outline your process and the appropriate modifications can be made :) edit: I should mention that this comparison allows for somebody to make the value of XMR lower so that the buyer attempting the edit cookies would need to pay more in XMR, but it should not allow somebody to increase the value of XMR (beyond 1 fiat denomination more than the live exchange rate) in an attempt to pay less in terms of XMR

evantbyrne commented 7 years ago

I saw the comment in the code and in my testing the other day it seemed it was indeed being limited to a max rate decrease of one fiat dollar per XMR, but I only tried injecting very basic numeric inputs. Storing this server-side would eliminate that potential attack vector altogether, and of course prevent customers from knocking the price down.

Cheers,

– Evan

serhack commented 7 years ago

Hello evan, Sorry for late response. I'm at holiday now. Thanks for discovering this security problem.

serhack commented 7 years ago

Solution proposed: Store value on database

cryptochangements34 commented 7 years ago

This issue should be fixed in Pull Request #13 which gets rid of the cookie and instead stores the exchange rate data in the MySQL database used by WordPress.

serhack commented 7 years ago

I can confirm that. Thanks to @evantbyrne and @cryptochangements34, plugin is safer than yesterday