sherlock-audit / 2024-06-makerdao-endgame-judging

1 stars 1 forks source link

zhoo - Some erc20 token transfer functions do not return a value #78

Closed sherlock-admin4 closed 1 month ago

sherlock-admin4 commented 1 month ago

zhoo

Medium

Some erc20 token transfer functions do not return a value

Summary

The VestedRewardsDistribution.distribute function needs to determine the return value of the gem.transfer function. However, the transfer function of some erc20 tokens (such as usdt) has no return value, resulting in a revert call to the distribute function.

Root Cause

VestedRewardsDistribution.distribute: https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/dba30d7a676c20dfed3bda8c52fd6702e2e85bb1/endgame-toolkit/src/VestedRewardsDistribution.sol#L161

    function distribute() external returns (uint256 amount) {
        require(vestId != INVALID_VEST_ID, "VestedRewardsDistribution/invalid-vest-id");

        amount = dssVest.unpaid(vestId);
        require(amount > 0, "VestedRewardsDistribution/no-pending-amount");

        lastDistributedAt = block.timestamp;
        dssVest.vest(vestId, amount);

@>   require(gem.transfer(address(stakingRewards), amount), "VestedRewardsDistribution/transfer-failed");
        stakingRewards.notifyRewardAmount(amount);

        emit Distribute(amount);
    }

If gem is usdt, usdt.transfer does not return a value:

https://etherscan.io/address/0xdac17f958d2ee523a2206206994597c13d831ec7#code

    function transfer(address _to, uint _value) public onlyPayloadSize(2 * 32) {
        uint fee = (_value.mul(basisPointsRate)).div(10000);
        if (fee > maximumFee) {
            fee = maximumFee;
        }
        uint sendAmount = _value.sub(fee);
        balances[msg.sender] = balances[msg.sender].sub(_value);
        balances[_to] = balances[_to].add(sendAmount);
        if (fee > 0) {
            balances[owner] = balances[owner].add(fee);
            Transfer(msg.sender, owner, fee);
        }
        Transfer(msg.sender, _to, sendAmount);
    }

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

  1. The gem VestedRewardsDistribution token is set non-standard erc20 token, such as usdt.
  2. The distribute function is called, but the transfer function has no return value, resulting in a revert, and the distribute call fails.

Impact

The distribute function failed to be called, and reward could not be distributed.

PoC

No response

Mitigation

Use safeTransfer instead of transfer

telome commented 1 month ago

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)."