sherlock-audit / 2024-06-magicsea-judging

8 stars 5 forks source link

PUSH0 - Protocol is incompatible with tokens that returns false on transfer #686

Closed sherlock-admin4 closed 3 months ago

sherlock-admin4 commented 3 months ago

PUSH0

Medium

Protocol is incompatible with tokens that returns false on transfer

Summary

Vulnerability Detail

OpenZeppelin's ERC20 safeTransfer forces that a token, if it returns a boolean on transferring, must return true.

There are some tokens that returns false even on successful transfers. Then using safeTransfer will always revert, and thus the protocol is incompatible with these tokens.

Some functionalities, such as MlumStaking.withdrawFromPosition(), or Masterchef.withdraw(), forces a reward harvesting when withdrawing. These functions will fail when the reward is inevitably transferred.

Impact

Protocol is incompatible with tokens that returns false on transfer

Code Snippet

https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/rewarders/BribeRewarder.sol#L120

https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/rewarders/BaseRewarder.sol#L306-L329

https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/MlumStaking.sol#L724-L731

Tool used

Manual Review

Recommendation

Consider handling the transfer separately if the protocol intends to support these tokens. Also consider adding an option to withdraw without harvesting rewards on Masterchef.