unRekt - `_claimRewardToken` function in `VaultRewarderLib` contract uses `IEIP20NonStandard` which makes protocol vulnerable to weird ERC20 behaviors.

[sherlock-admin2]

unRekt

Medium

_claimRewardToken function in VaultRewarderLib contract uses IEIP20NonStandard which makes protocol vulnerable to weird ERC20 behaviors.

Summary

The _claimRewardToken function in VaultRewarderLib contract uses IEIP20NonStandard , which is an unsafe version of ERC20 implementation that can lead to vulnerabilities.

Vulnerability Detail

The _claimRewardToken function in VaultRewarderLib contract uses IEIP20NonStandard , which is an unsafe version of ERC20 implementation without safety checks. The library clearly mentions on the top that "Version of ERC20 with no return values for transfer and transferFrom" and also links an article with 130 different tokens affected due to this vulnerability here but still it is being used by the protocol.

The comment above the function -

            // Ignore transfer errors here so that any strange failures here do not
            // prevent normal vault operations from working. Failures may include a
            // lack of balances or some sort of blacklist that prevents an account
            // from receiving tokens.

Link to code - here

The comment shows that the protocol is making and assumption that we should ignore transfer errors and any strange failure and it doesn't prevent normal functioning of the vault. If we consider that assumption as correct, then there should be checks present to ensure using IEIP20NonStandard is safe.

In the IEIP20NonStandard contract it is clearly mentioned above the transfer function -

  /// !!! NOTICE !!! `transfer` does not return a value, in violation of the ERC-20 specification

While this can be considered as a design choice , only viable reason to use an non-standard version of ERC20 implementation than the standard one, can be the non-standard version provides certain features that the standard and safe version doesn't.

Impact

Using non-standard version of ERC20 implementation can lead to protocol encountering unexpected failure and give bad user experience.

Code Snippet

Tool used

Manual Review

Recommendation

Use OpenZeppelin's SafeERC20 versions with the safeTransfer and safeTransferFrom functions Handle non-standard-compliant tokens

[sherlock-admin2]

1 comment(s) were left on this issue during the judging contest.

0xmystery commented:

Low/QA at most