search

Cyfrin / 2023-07-beedle

21 stars 20 forks source link

The owner of the protocol can frontrun the `setLenderFee` if some borrower borrows a big amount of tokens #564

Open codehawks-bot opened 1 year ago

codehawks-bot commented 1 year ago

The owner of the protocol can frontrun the setLenderFee if some borrower borrows a big amount of tokens

Severity

High Risk

Relevant GitHub Links

https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L265

Summary

The protocol exhibits a vulnerability where the owner has the ability to front-run the setLenderFee function when a borrower attempts to borrow a significant amount of tokens. As the FeeReceiver, the owner can exploit this situation to manipulate the fees by setting them to their maximum value. Consequently, the owner gains control over the user's tokens.

Vulnerability Details

When a borrower seeks to borrow a substantial number of tokens, and the owner of the protocol also acts as the FeeReceiver, a scenario arises in which the owner can preempt the borrower's transaction. By front-running the borrower's action, the owner can modify the fees, setting them to the maximum possible value. This action allows the owner to take control of the user's tokens.

Impact

The vulnerability poses a severe risk of fund loss and undermines the trustworthiness of the protocol. If exploited, users may suffer financial losses, eroding confidence in the platform.

Tools Used

The identified vulnerability was discovered through manual review of the protocol's codebase.

Recommendations

To address this vulnerability and prevent potential exploitation, it is advisable to introduce a time delay mechanism for the setLenderFee function. By updating the fees after a predetermined duration, for example, using block.timestamp + 5 days, the protocol can create a buffer period during which fee adjustments cannot be modified instantaneously. This time delay mechanism adds an additional layer of protection, mitigating the risk of front-running and unauthorized fee manipulation by the owner.

PatrickAlphaC commented 1 year ago

I disagree with the recommendation. I think an expected fee should be passed in with calls that interact with the fee.

The likelihood of this is LOW due to a trusted role but the impact is HIGH so I will mark this as a medium.

abarbatei commented 1 year ago

Hello, issue is related to owner/centralization risk which is mentioned in known issues https://github.com/Cyfrin/2023-07-beedle/issues/2#M-1 Could you please recheck?

kosedogus commented 1 year ago

This is both a known issue and also sponsor mentions that owner is trusted. Hence should be invalid.

PatrickAlphaC commented 1 year ago

Gah. I guess technically you're right... This is important to call out to the protocol though. Moving to info.

On my live stream, I argued that this was so egregious that it needs to be called out... but in fairness it is a "known" issue... I will update it to info.