code-423n4 / 2022-02-concur-findings

2 stars 0 forks source link

Owner can lock tokens in `MasterChef` #238

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-02-concur/blob/main/contracts/MasterChef.sol#L82-L84

Vulnerability details

Impact

Owner can remove a depositor. Since only depositors can deposit and withdraw, the owner may add a contract to the whitelist, let users deposit in the contarct and remove the depositor from the whitelist. Depositor's reward cannot be withdrawn then. And takes a share of Concur tokens that will not be ditributed.

Tools Used

Manual analysis

Recommended Mitigation Steps

Remove onlyDepositor modifier from the withdraw function.

GalloDaSballo commented 2 years ago

The finding is valid in that the sponsor / owner can remove all depositors.

I believe having an immutable depositor that can't be changed would give stronger security guarantees.

While the finding is valid, because it is contingent on a malicious owner, I believe Medium Severity to be more appropriate