JuggerNaut63 - Token Transfer Failure Due to Non-Standard ERC20 Implementation

[sherlock-admin3]

JuggerNaut63

Medium

Token Transfer Failure Due to Non-Standard ERC20 Implementation

Summary

The gem.transfer function is used to transfer tokens in VestedRewardsDistribution. StakingRewards does not have a function that returns a boolean value, this can lead to unexpected behavior and potential failures in the token transfer process. https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/main/endgame-toolkit/src/VestedRewardsDistribution.sol#L161

Vulnerability Detail

The gem.transfer function is called in the distributed function to transfer tokens from VestedRewardsDistribution to StakingRewards. The ERC20 standard specifies that the transfer function must return a boolean value indicating the success of the transfer. However, not all ERC20 token implementations adhere strictly to this standard. Some tokens may revert to their original state on failure or return nothing, which can cause the gem.transfer call to behave unexpectedly.

Impact

• Silent Failures
• Security Risks
• Operational Issues

Code Snippet

https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/main/endgame-toolkit/src/VestedRewardsDistribution.sol#L152-L165

Tool used

Manual Review

Recommendation

Use OpenZeppelin's SafeERC20 library to handle token transfers. The safeTransfer function provided by this library ensures that transfers succeed by correctly handling non-standard ERC20 implementations.

[telome]

From the competition rules: "The token contracts used in each repository are known in advance and there is no use of arbitrary tokens in any of the contest modules: [...] In endgame-toolkit the farm types to be used through different deployment phases are (using rewards/stake notation): NGT/NST, SDAO/NST, NGT/SDAO, NST/LSMKR and SDAO/LSMKR (the last two types are only for lockstake)."