code-423n4 / 2023-05-maia-findings

20 stars 12 forks source link

Although `ERC20Boost.transfer` and `ERC20Boost.transferFrom` functions try to prevent sender from transferring her or his gauge boost amount that is not free to receiver, such sender can still call `UtilityManager.forfeitBoost` and `bHermes.transfer` or `bHermes.transferFrom` functions to bypass such prevention #910

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-05-maia/blob/62f4f01a522dcbb4b9abfe2f6783bbb67c0da022/src/erc-20/ERC20Boost.sol#L116-L135 https://github.com/code-423n4/2023-05-maia/blob/62f4f01a522dcbb4b9abfe2f6783bbb67c0da022/src/erc-20/ERC20Boost.sol#L81-L83 https://github.com/code-423n4/2023-05-maia/blob/62f4f01a522dcbb4b9abfe2f6783bbb67c0da022/src/erc-20/ERC20Boost.sol#L312-L314 https://github.com/code-423n4/2023-05-maia/blob/62f4f01a522dcbb4b9abfe2f6783bbb67c0da022/src/erc-20/ERC20Boost.sol#L323-L330 https://github.com/code-423n4/2023-05-maia/blob/62f4f01a522dcbb4b9abfe2f6783bbb67c0da022/src/erc-20/ERC20Boost.sol#L341-L344 https://github.com/code-423n4/2023-05-maia/blob/53c7fe9d5e55754960eafe936b6cb592773d614c/src/hermes/UtilityManager.sol#L78-L84 https://github.com/code-423n4/2023-05-maia/blob/62f4f01a522dcbb4b9abfe2f6783bbb67c0da022/src/hermes/bHermes.sol#L140-L149 https://github.com/code-423n4/2023-05-maia/blob/62f4f01a522dcbb4b9abfe2f6783bbb67c0da022/src/hermes/bHermes.sol#L158-L167

Vulnerability details

Impact

After calling the following ERC20Boost.attach function to attach a user's boost to a gauge, such user's getUserBoost[user] is updated to the corresponding balanceOf[user].toUint128(). Since getUserBoost[user] for such user becomes higher, the value returned by the ERC20Boost.freeGaugeBoost function below becomes lower.

https://github.com/code-423n4/2023-05-maia/blob/62f4f01a522dcbb4b9abfe2f6783bbb67c0da022/src/erc-20/ERC20Boost.sol#L116-L135

    function attach(address user) external {
        if (!_gauges.contains(msg.sender) || _deprecatedGauges.contains(msg.sender)) {
            revert InvalidGauge();
        }

        // idempotent add
        if (!_userGauges[user].add(msg.sender)) revert GaugeAlreadyAttached();

        uint128 userGaugeBoost = balanceOf[user].toUint128();

        if (getUserBoost[user] < userGaugeBoost) {
            getUserBoost[user] = userGaugeBoost;
            emit UpdateUserBoost(user, userGaugeBoost);
        }

        getUserGaugeBoost[user][msg.sender] =
            GaugeState({userGaugeBoost: userGaugeBoost, totalGaugeBoost: totalSupply.toUint128()});

        emit Attach(user, msg.sender, userGaugeBoost);
    }

https://github.com/code-423n4/2023-05-maia/blob/62f4f01a522dcbb4b9abfe2f6783bbb67c0da022/src/erc-20/ERC20Boost.sol#L81-L83

    function freeGaugeBoost(address user) public view returns (uint256) {
        return balanceOf[user] - getUserBoost[user];
    }

Because the following ERC20Boost.transfer and ERC20Boost.transferFrom functions call the ERC20Boost.notAttached modifier below, the previously mentioned user is only able to transfer up to the boost amount returned by the ERC20Boost.freeGaugeBoost function to the to address. This prevents such user from sending her or his gauge boost amount that is not free.

https://github.com/code-423n4/2023-05-maia/blob/62f4f01a522dcbb4b9abfe2f6783bbb67c0da022/src/erc-20/ERC20Boost.sol#L312-L314

    function transfer(address to, uint256 amount) public override notAttached(msg.sender, amount) returns (bool) {
        return super.transfer(to, amount);
    }

https://github.com/code-423n4/2023-05-maia/blob/62f4f01a522dcbb4b9abfe2f6783bbb67c0da022/src/erc-20/ERC20Boost.sol#L323-L330

    function transferFrom(address from, address to, uint256 amount)
        public
        override
        notAttached(from, amount)
        returns (bool)
    {
        return super.transferFrom(from, to, amount);
    }

https://github.com/code-423n4/2023-05-maia/blob/62f4f01a522dcbb4b9abfe2f6783bbb67c0da022/src/erc-20/ERC20Boost.sol#L341-L344

    modifier notAttached(address user, uint256 amount) {
        if (freeGaugeBoost(user) < amount) revert AttachedBoost();
        _;
    }

However, such user can call the following UtilityManager.forfeitBoost function to reduce her or his userClaimedBoost and then call the bHermes.transfer or bHermes.transferFrom function below to transfer a bHermes token amount, which equals such user's gauge boost amount that is not free, to the to address.

https://github.com/code-423n4/2023-05-maia/blob/53c7fe9d5e55754960eafe936b6cb592773d614c/src/hermes/UtilityManager.sol#L78-L84

    function forfeitBoost(uint256 amount) public virtual {
        if (amount == 0) return;
        userClaimedBoost[msg.sender] -= amount;
        address(gaugeBoost).safeTransferFrom(msg.sender, address(this), amount);

        emit ForfeitBoost(msg.sender, amount);
    }

https://github.com/code-423n4/2023-05-maia/blob/62f4f01a522dcbb4b9abfe2f6783bbb67c0da022/src/hermes/bHermes.sol#L140-L149

    function transfer(address to, uint256 amount) public virtual override returns (bool) {
        uint256 userBalance = balanceOf[msg.sender];

        if (
            userBalance - userClaimedWeight[msg.sender] < amount || userBalance - userClaimedBoost[msg.sender] < amount
                || userBalance - userClaimedGovernance[msg.sender] < amount
        ) revert InsufficientUnderlying();

        return super.transfer(to, amount);
    }

https://github.com/code-423n4/2023-05-maia/blob/62f4f01a522dcbb4b9abfe2f6783bbb67c0da022/src/hermes/bHermes.sol#L158-L167

    function transferFrom(address from, address to, uint256 amount) public virtual override returns (bool) {
        uint256 userBalance = balanceOf[from];

        if (
            userBalance - userClaimedWeight[from] < amount || userBalance - userClaimedBoost[from] < amount
                || userBalance - userClaimedGovernance[from] < amount
        ) revert InsufficientUnderlying();

        return super.transferFrom(from, to, amount);
    }

Afterwards, the receiving user corresponding to the to address can call the following UtilityManager.claimBoost function to use the received bHermes token amount to claim the boost amount, which equals the sending user's gauge boost amount that is not free. As a result, although the ERC20Boost.transfer and ERC20Boost.transferFrom functions try to prevent the sending user from transferring her or his gauge boost amount that is not free to the receiving user, such sending user can still call the UtilityManager.forfeitBoost and bHermes.transfer or bHermes.transferFrom functions to bypass such prevention.

https://github.com/code-423n4/2023-05-maia/blob/53c7fe9d5e55754960eafe936b6cb592773d614c/src/hermes/UtilityManager.sol#L119-L125

    function claimBoost(uint256 amount) public virtual checkBoost(amount) {
        if (amount == 0) return;
        userClaimedBoost[msg.sender] += amount;
        address(gaugeBoost).safeTransfer(msg.sender, amount);

        emit ClaimBoost(msg.sender, amount);
    }

https://github.com/code-423n4/2023-05-maia/blob/62f4f01a522dcbb4b9abfe2f6783bbb67c0da022/src/hermes/bHermes.sol#L77-L82

    modifier checkBoost(uint256 amount) override {
        if (balanceOf[msg.sender] < amount + userClaimedBoost[msg.sender]) {
            revert InsufficientShares();
        }
        _;
    }

Proof of Concept

The following steps can occur for the described scenario.

  1. Alice and Bob collude.
  2. Alice has 1e18 bHermes and claimed 1e18 gauge boost.
  3. All of Alice's 1e18 gauge boost are attached to a gauge.
  4. Alice calls the UtilityManager.forfeitBoost function with the amount input being 1e18 to reduce her userClaimedBoost to 0.
  5. Alice calls the bHermes.transfer function to transfer 1e18 bHermes to Bob.
  6. Bob calls the UtilityManager.claimBoost function to claim 1e18 gauge boost using the received 1e18 bHermes even though Alice's 1e18 gauge boost are not free.

Tools Used

VSCode

Recommended Mitigation Steps

The UtilityManager.forfeitBoost function can be updated to only allow msg.sender to forfeit up to her or his gauge boost amount that is free. In other words, calling this function with the amount input being more than msg.sender's gauge boost amount that is free should revert.

Assessed type

Other

c4-judge commented 1 year ago

trust1995 changed the severity to QA (Quality Assurance)

c4-judge commented 1 year ago

trust1995 marked the issue as grade-c