4b - Unhandled return value of transfer can cause `EthenaCooldownHolder::_finalizeCooldown()` to return true when it is not supposed to

[sherlock-admin4]

4b

Medium

Unhandled return value of transfer can cause EthenaCooldownHolder::_finalizeCooldown() to return true when it is not supposed to

Summary

In EthenaCooldownHolder::_finalizeCooldown() the return value of the transfer function is not handled which can lead to finalized value being true when there was no transfer

Vulnerability Detail

ERC20 implementations are not always consistent. Some implementations of transfer and transferFrom could return ‘false’ on failure instead of reverting. so it is always advised to handle these reverts to prevent further complication

Impact

In EthenaCooldownHolder::_finalizeCooldown() the finalized value can be set to true when the transfer returns a false value instead of a revert

Code Snippet

EthenaCooldownHolder::_finalizeCooldown()

   function _finalizeCooldown() internal override returns (uint256 tokensClaimed, bool finalized) {
        uint24 duration = sUSDe.cooldownDuration();
        IsUSDe.UserCooldown memory userCooldown = sUSDe.cooldowns(address(this));

        if (block.timestamp < userCooldown.cooldownEnd && 0 < duration) {
            // Cooldown has not completed, return a false for finalized
            return (0, false);
        }

        // If a cooldown has been initiated, need to call unstake to complete it. If
        // duration was set to zero then the USDe will be on this contract already.
        if (0 < userCooldown.cooldownEnd) sUSDe.unstake(address(this));

        // USDe is immutable. It cannot have a transfer tax and it is ERC20 compliant
        // so we do not need to use the additional protections here.
        tokensClaimed = USDe.balanceOf(address(this));
        //@audit
        USDe.transfer(vault, tokensClaimed);
        //@audit this value can be set to true if transfer returns false and does not revert
        finalized = true;
    }

Tool used

Manual Review

Recommendation

Use safeTransfer or handle reverts

[sherlock-admin3]

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

0xmystery commented:

Low/QA at most