search

code-423n4 / 2021-10-tally-findings

0 stars 0 forks source link

timelocked governance can grief with the fees #50

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

pauliax

Vulnerability details

Impact

swapFee_ in setSwapFee can be set arbitrarily high, up to 100%, so in theory, governance can monitor the mempool and frontrun the txs. A similar issue was submitted in a previous contest and assigned a severity of medium: https://github.com/code-423n4/2021-05-nftx-findings/issues/51

Recommended Mitigation Steps

Consider either introducing a reasonable upper limit (e.g. 15%) or making changes to the code that this new fee will only apply starting from the next block or something like that.

Shadowfiend commented 2 years ago

If the update is timelocked, it's not clear this is a real risk. Certainly governance can introduce a timelocked change that bumps the fee and wait to send it through, but the point of the timelock is to give folks the opportunity to observe and react to the change.

Letting this coexist with #10 as it comes at it from a slightly different direction, but unsure on whether we'll try to mitigate.

0xean commented 2 years ago

agree with sponsor that the upper limit is probably the better take away from this issue and therefore a dupe of #10

0xean commented 2 years ago

Modifying severity to align with #10