Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/tokens/GuildToken.sol#L41

Vulnerability details

Impact

The whole protocol can only support one asset instead of multi assets they claimed.

Proof of Concept

ECG protocol claimed that it supports multi assets in its introduction:

The Credit Guild is a protocol for trust minimized pooled lending on the Ethereum network. Its goal is to scale governance to support a larger set of assets and loan terms, while minimizing third party risk.

It is also confirmed in contest introduction:

The first deployment of the protocol only has one credit token pegged to USDC: gUSDC ("Guild USDC"). In the future, it is expected that multiple markets will be deployed, each with their own credit token, but the GUILD token is the same for all markets.

It's obvious that only one GUILD token will be deployed to govern all markets(gUSDC, gDAI, gETH...)

Yet, the protocol lacks this ability owing to a design flaw:

GuildToken is the governance token of the Ethereum Credit Guild, which means that only one GuildToken will be deployed for ECG protocol.
Only a single profitManager is defined in GuildToken. https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/tokens/GuildToken.sol#L41:
```
40:    /// @notice reference to ProfitManager
41:    address public profitManager;
```
There is only one credit defined in ProfitManager. https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/governance/ProfitManager.sol#L35:
```
34:    /// @notice reference to CREDIT token.
35:    address public credit;
```
From above we can see, ECG protocol can only support one credit token, which is defined in ProfitManager.

Tools Used

Manual review

Recommended Mitigation Steps

Allow GuildToken to support multi profitManager:

// SPDX-License-Identifier: GPL-3.0-or-later
pragma solidity 0.8.13;

import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import {ERC20Permit} from "@openzeppelin/contracts/token/ERC20/extensions/ERC20Permit.sol";
import {ERC20Burnable} from "@openzeppelin/contracts/token/ERC20/extensions/ERC20Burnable.sol";
import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol";

import {CoreRef} from "@src/core/CoreRef.sol";
import {CoreRoles} from "@src/core/CoreRoles.sol";
import {LendingTerm} from "@src/loan/LendingTerm.sol";
import {ERC20Gauges} from "@src/tokens/ERC20Gauges.sol";
import {ProfitManager} from "@src/governance/ProfitManager.sol";
import {ERC20MultiVotes} from "@src/tokens/ERC20MultiVotes.sol";

/** 
@title  GUILD ERC20 Token
@author eswak
@notice This is the governance token of the Ethereum Credit Guild.
    On deploy, this token is non-transferrable.
    During the non-transferrable period, GUILD can still be minted & burnt, only
    `transfer` and `transferFrom` are reverting.

    The gauge system is used to define debt ceilings on a set of lending terms.
    Lending terms can be whitelisted by adding a gauge for their address, if GUILD
    holders vote for these lending terms in the gauge system, the lending terms will
    have a non-zero debt ceiling, and borrowing will be available under these terms.

    When a lending term creates bad debt, a loss is notified in a gauge on this
    contract (`notifyGaugeLoss`). When a loss is notified, all the GUILD token weight voting
    for this gauge becomes non-transferable and can be permissionlessly slashed. Until the
    loss is realized (`applyGaugeLoss`), a user cannot transfer their locked tokens or
    decrease the weight they assign to the gauge that suffered a loss.
    Even when a loss occur, users can still transfer tokens with which they vote for gauges
    that did not suffer a loss.
*/
contract GuildToken is CoreRef, ERC20Burnable, ERC20Gauges, ERC20MultiVotes {
    using EnumerableSet for EnumerableSet.AddressSet;

    /// @notice reference to ProfitManager
-   address public profitManager;
+   mapping(uint256 => address) public profitManagers;

    constructor(
+       address _core
-       address _core,
-       address _profitManager
    )
        CoreRef(_core)
        ERC20("Ethereum Credit Guild - GUILD", "GUILD")
        ERC20Permit("Ethereum Credit Guild - GUILD")
    {
-       profitManager = _profitManager;
    }

    /*///////////////////////////////////////////////////////////////
                        VOTING MANAGEMENT
    //////////////////////////////////////////////////////////////*/

    /// @notice Set `maxDelegates`, the maximum number of addresses any account can delegate voting power to.
    function setMaxDelegates(
        uint256 newMax
    ) external onlyCoreRole(CoreRoles.GUILD_GOVERNANCE_PARAMETERS) {
        _setMaxDelegates(newMax);
    }

    /// @notice Allow or disallow an address to delegate voting power to more addresses than `maxDelegates`.
    function setContractExceedMaxDelegates(
        address account,
        bool canExceedMax
    ) external onlyCoreRole(CoreRoles.GUILD_GOVERNANCE_PARAMETERS) {
        _setContractExceedMaxDelegates(account, canExceedMax);
    }

    /*///////////////////////////////////////////////////////////////
                        GAUGE MANAGEMENT
    //////////////////////////////////////////////////////////////*/
    function addGauge(
        uint256 _type,
        address gauge
    ) external onlyCoreRole(CoreRoles.GAUGE_ADD) returns (uint256) {
        return _addGauge(_type, gauge);
    }

    function removeGauge(
        address gauge
    ) external onlyCoreRole(CoreRoles.GAUGE_REMOVE) {
        _removeGauge(gauge);
    }

    function setMaxGauges(
        uint256 max
    ) external onlyCoreRole(CoreRoles.GAUGE_PARAMETERS) {
        _setMaxGauges(max);
    }

    function setCanExceedMaxGauges(
        address who,
        bool can
    ) external onlyCoreRole(CoreRoles.GAUGE_PARAMETERS) {
        _setCanExceedMaxGauges(who, can);
    }

    /*///////////////////////////////////////////////////////////////
                        LOSS MANAGEMENT
    //////////////////////////////////////////////////////////////*/

    /// @notice emitted when a loss in a gauge is notified.
    event GaugeLoss(address indexed gauge, uint256 indexed when);
    /// @notice emitted when a loss in a gauge is applied (for each user).
    event GaugeLossApply(
        address indexed gauge,
        address indexed who,
        uint256 weight,
        uint256 when
    );

    /// @notice last block.timestamp when a loss occurred in a given gauge
    mapping(address => uint256) public lastGaugeLoss;

    /// @notice last block.timestamp when a user apply a loss that occurred in a given gauge
    mapping(address => mapping(address => uint256)) public lastGaugeLossApplied;

    /// @notice notify loss in a given gauge
    function notifyGaugeLoss(address gauge) external {
-       require(msg.sender == profitManager, "UNAUTHORIZED");
+       require(msg.sender == profitManagers[gaugeType[gauge]], "UNAUTHORIZED");

        // save gauge loss
        lastGaugeLoss[gauge] = block.timestamp;
        emit GaugeLoss(gauge, block.timestamp);
    }

    /// @notice apply a loss that occurred in a given gauge
    /// anyone can apply the loss on behalf of anyone else
    function applyGaugeLoss(address gauge, address who) external {
        // check preconditions
        uint256 _lastGaugeLoss = lastGaugeLoss[gauge];
        uint256 _lastGaugeLossApplied = lastGaugeLossApplied[gauge][who];
        require(
            _lastGaugeLoss != 0 && _lastGaugeLossApplied < _lastGaugeLoss,
            "GuildToken: no loss to apply"
        );

        // read user weight allocated to the lossy gauge
        uint256 _userGaugeWeight = getUserGaugeWeight[who][gauge];

        // remove gauge weight allocation
        lastGaugeLossApplied[gauge][who] = block.timestamp;
        _decrementGaugeWeight(who, gauge, _userGaugeWeight);
        if (!_deprecatedGauges.contains(gauge)) {
            totalTypeWeight[gaugeType[gauge]] -= _userGaugeWeight;
            totalWeight -= _userGaugeWeight;
        }

        // apply loss
        _burn(who, uint256(_userGaugeWeight));
        emit GaugeLossApply(
            gauge,
            who,
            uint256(_userGaugeWeight),
            block.timestamp
        );
    }

    /*///////////////////////////////////////////////////////////////
                        TRANSFERABILITY
    //////////////////////////////////////////////////////////////*/

    /// @notice at deployment, tokens are not transferable (can only mint/burn).
    /// Governance can enable transfers with `enableTransfers()`.
    bool public transferable; // default = false

    /// @notice emitted when transfers are enabled.
    event TransfersEnabled(uint256 block, uint256 timestamp);

    /// @notice permanently enable token transfers.
    function enableTransfer() external onlyCoreRole(CoreRoles.GOVERNOR) {
        transferable = true;
        emit TransfersEnabled(block.number, block.timestamp);
    }

    /// @dev prevent transfers if they are not globally enabled.
    /// mint and burn (transfers to and from address 0) are accepted.
    function _beforeTokenTransfer(
        address from,
        address to,
        uint256 /* amount*/
    ) internal view override {
        require(
            transferable || from == address(0) || to == address(0),
            "GuildToken: transfers disabled"
        );
    }

    /// @notice emitted when reference to ProfitManager is updated
    event ProfitManagerUpdated(uint256 timestamp, address newValue);

    /// @notice set reference to ProfitManager
-   function setProfitManager(address _newProfitManager) external onlyCoreRole(CoreRoles.GOVERNOR) {
-       profitManager = _newProfitManager;
+   function setProfitManager(uint256 _type, address _newProfitManager) external onlyCoreRole(CoreRoles.GOVERNOR) {
+       profitManagers[_type] = _newProfitManager;
        emit ProfitManagerUpdated(block.timestamp, _newProfitManager);
    }

    /// @dev prevent outbound token transfers (_decrementWeightUntilFree) and gauge weight decrease
    /// (decrementGauge, decrementGauges) for users who have an unrealized loss in a gauge, or if the
    /// gauge is currently using its allocated debt ceiling. To decrement gauge weight, guild holders
    /// might have to call loans if the debt ceiling is used.
    /// Also update the user profit index and claim rewards.
    function _decrementGaugeWeight(
        address user,
        address gauge,
        uint256 weight
    ) internal override {
        uint256 _lastGaugeLoss = lastGaugeLoss[gauge];
        uint256 _lastGaugeLossApplied = lastGaugeLossApplied[gauge][user];
        require(
            _lastGaugeLossApplied >= _lastGaugeLoss,
            "GuildToken: pending loss"
        );

        // update the user profit index and claim rewards
-       ProfitManager(profitManager).claimGaugeRewards(user, gauge);
+       ProfitManager(profitManagers[gaugeType[gauge]]).claimGaugeRewards(user, gauge);

        // check if gauge is currently using its allocated debt ceiling.
        // To decrement gauge weight, guild holders might have to call loans if the debt ceiling is used.
        uint256 issuance = LendingTerm(gauge).issuance();
        if (issuance != 0) {
            uint256 debtCeilingAfterDecrement = LendingTerm(gauge).debtCeiling(-int256(weight));
            require(
                issuance <= debtCeilingAfterDecrement,
                "GuildToken: debt ceiling used"
            );
        }

        super._decrementGaugeWeight(user, gauge, weight);
    }

    /// @dev prevent weight increment for gauge if user has an unapplied loss.
    /// If the user has 0 weight (i.e. no loss to realize), allow incrementing
    /// gauge weight & update lastGaugeLossApplied to current time.
    /// Also update the user profit index an claim rewards.
    /// @dev note that users voting for a gauge that is not a proper lending term could result in this
    /// share of the user's tokens to be frozen, due to being unable to decrement weight.
    function _incrementGaugeWeight(
        address user,
        address gauge,
        uint256 weight
    ) internal override {
        uint256 _lastGaugeLoss = lastGaugeLoss[gauge];
        uint256 _lastGaugeLossApplied = lastGaugeLossApplied[gauge][user];
        if (getUserGaugeWeight[user][gauge] == 0) {
            lastGaugeLossApplied[gauge][user] = block.timestamp;
        } else {
            require(
                _lastGaugeLossApplied >= _lastGaugeLoss,
                "GuildToken: pending loss"
            );
        }
-       ProfitManager(profitManager).claimGaugeRewards(user, gauge);
+       ProfitManager(profitManagers[gaugeType[gauge]]).claimGaugeRewards(user, gauge);

        super._incrementGaugeWeight(user, gauge, weight);
    }

    /*///////////////////////////////////////////////////////////////
                        MINT / BURN
    //////////////////////////////////////////////////////////////*/

    /// @notice mint new tokens to the target address
    function mint(
        address to,
        uint256 amount
    ) external onlyCoreRole(CoreRoles.GUILD_MINTER) {
        _mint(to, amount);
    }

    /*///////////////////////////////////////////////////////////////
                        Inheritance reconciliation
    //////////////////////////////////////////////////////////////*/

    function _burn(
        address from,
        uint256 amount
    ) internal virtual override(ERC20, ERC20Gauges, ERC20MultiVotes) {
        _decrementWeightUntilFree(from, amount);
        _decrementVotesUntilFree(from, amount);
        ERC20._burn(from, amount);
    }

    function transfer(
        address to,
        uint256 amount
    )
        public
        virtual
        override(ERC20, ERC20Gauges, ERC20MultiVotes)
        returns (bool)
    {
        _decrementWeightUntilFree(msg.sender, amount);
        _decrementVotesUntilFree(msg.sender, amount);
        return ERC20.transfer(to, amount);
    }

    function transferFrom(
        address from,
        address to,
        uint256 amount
    )
        public
        virtual
        override(ERC20, ERC20Gauges, ERC20MultiVotes)
        returns (bool)
    {
        _decrementWeightUntilFree(from, amount);
        _decrementVotesUntilFree(from, amount);
        return ERC20.transferFrom(from, to, amount);
    }
}

Assessed type

Other

code-423n4 / 2023-12-ethereumcreditguild-findings

`GuildToken` lack support for multiple lending assets as it only define a single `profitManager` #648