Shadowfiend
commented
3 years ago We may try to update this to set a max fee, but at the same time if governance tries to steal all swap value as a fee we expect folks will vote with their feet.
Open code423n4 opened 3 years ago
Shadowfiend
commented
3 years ago We may try to update this to set a max fee, but at the same time if governance tries to steal all swap value as a fee we expect folks will vote with their feet.
Handle
TomFrench
Vulnerability details
Impact
Swap fee could potentially be set to 100% resulting in total loss of funds for users in the case of malicious/faulty governor contract.
Proof of Concept
Swap.sol allows setSwapFee to be called by the address listed as the timelock by its governor contract. It's clear that this is meant to prevent frontrunning of swaps by the governor. No governor/timelock contract is in scope but should these be malicious/faulty it may be possible for this timelock behaviour to be circumvented.
Recommended Mitigation Steps
Set a constant maximum value which you never expect the swap fee to exceed (e.g. a swap fee of 10% would discourage users from performing any swaps) This ensures that even in the case of the timelock failing then users are guaranteed to retain at least 90% of their tokens.
There's no real reason to allow a 100% fee so by capping the max fee to a more reasonable value concerns about the governance contracts can be mitigated.