The MasterChefV2 contract allows for the creation of pools with any ERC-20 token, including fee-on-transfer tokens. These tokens transfer less than the specified amount due to a fee, resulting in users being credited with more funds than actually deposited. This discrepancy allows users to withdraw more than they deposited, potentially leaving the last user with nothing.
Vulnerability Detail
The MasterchefV2 contract allows for the creation of pools with all ERC-20 tokens, as well as weird tokens.
If you are integrating tokens, are you allowing only whitelisted tokens to work with the codebase or any complying with the standard? Are they assumed to have certain properties, e.g. be non-reentrant? Are there any types of [weird tokens](https://github.com/d-xo/weird-erc20) you want to integrate?
Any type of ERC20 token. Pools are permissionless. So users can open pools even with weird tokens. Issues regarding any weird token will be valid if they have Med/High impact.
This also includes fee-on-transfer tokens. These kind of tokens will transfer less than the amount on a transfer and keep a fee. As a result the user will be credited more funds than intended in the MasterChefV2 contract. The will also be able to withdraw these later, which can lead to a situation where the last user to withdraw won't get anything out anymore because all others withdraw more than they put in.
Impact
The issue results in the user getting credited more deposits in the pool than was actually deposited. As a result the user will also be able to withdraw more than actually deposited and the last users deposits will be stolen.
Reentrants
High
MasterChefV2 does not account for FOT tokens
Summary
The MasterChefV2 contract allows for the creation of pools with any ERC-20 token, including fee-on-transfer tokens. These tokens transfer less than the specified amount due to a fee, resulting in users being credited with more funds than actually deposited. This discrepancy allows users to withdraw more than they deposited, potentially leaving the last user with nothing.
Vulnerability Detail
The MasterchefV2 contract allows for the creation of pools with all ERC-20 tokens, as well as weird tokens.
This also includes fee-on-transfer tokens. These kind of tokens will transfer less than the amount on a transfer and keep a fee. As a result the user will be credited more funds than intended in the
MasterChefV2contract. The will also be able to withdraw these later, which can lead to a situation where the last user to withdraw won't get anything out anymore because all others withdraw more than they put in.Impact
The issue results in the user getting credited more deposits in the pool than was actually deposited. As a result the user will also be able to withdraw more than actually deposited and the last users deposits will be stolen.
Code Snippet
https://github.com/sherlock-audit/2024-06-magicsea/blob/42e799446595c542eff9519353d3becc50cdba63/magicsea-staking/src/MasterchefV2.sol#L284-L288
https://github.com/sherlock-audit/2024-06-magicsea/blob/42e799446595c542eff9519353d3becc50cdba63/magicsea-staking/src/MasterchefV2.sol#L295-L299
Tool used
Manual Review
Recommendation
We recommend checking the balance before and after a deposit to a pool and only crediting the difference to the user.
Duplicate of #545