code-423n4 / 2023-06-reserve-findings

1 stars 0 forks source link

Dutch auction is costly for bidder which means that system will likely receive less assets then expected #44

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/reserve-protocol/protocol/blob/c4ec2473bbcb4831d62af55d275368e73e16b984/contracts/plugins/trading/DutchTrade.sol#L146-L164

Vulnerability details

Impact

Dutch auction is costly for bidder which means that system will likely receive less assets then expected.

Proof of Concept

When BackingManager.rebalance is called then system calculates the best assets to trade to cover deficit. In case if Dutch trade is chosen, that means that price for assets will be set high and then during the time it will be going down.

When someone agrees to buy assets, he should call bid function. This will settle BackingManager, which will try to rebalance again. Because of this, bidding is not cheap for the user. Depending on amount of assets that should be checked for optimistic/pessimistic cases, gas amount consumed by function can be very big. That means that users will not buy traded assets for the market price(as it was calculated in recollateralization lib), they will also account for gas price when bidding. That means that they will actually pay less amount to the RToken system and recollateralization will be less efficient, especially when main net is busy and gas price is high. This will have bad impact on the whole system.

Tools Used

VsCode

Recommended Mitigation Steps

I guess that Dutch auction is not efficient, maybe it's not needed. Or you can not call rebalance when settling. Let user pay for bidding and closing trade. And someone else will start another cycle of rebalancing. Then prices, received for auction will be more predictable.

Assessed type

Error

c4-judge commented 1 year ago

0xean changed the severity to QA (Quality Assurance)