SurplusGuildMinter define MIN_STAKE constant to 1e18, which is large amount for minimum, if protocol wants to support gWETH

[c4-bot-8]

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/loan/SurplusGuildMinter.sol#L26

Vulnerability details

Impact

The value uint256 public constant MIN_STAKE = 1e18; inside SurplusGuildMinter is hardcoded based on USDC (1 USD as minimum), which is a rational decision, but as the sponsors has confirmed the protocol plan to support other credit tokens such as gWETH, which minimum stake amount would cost > $2300 (in time of writing this report 1 ETH = $2300), which is 230000% value difference between gUSDC SurplusGuildMinter and gWETH SurplusGuildMinter.

Proof of Concept

Sponsor has confirmed that this is a valid concern and a code change should be applied based on this finding.

Tools Used

Manual Review

Recommended Mitigation Steps

Don't hardcode the MIN_STAKE inside SurplusGuildMinter. Instead, pass it as a constructor parameter:

   /// @notice minimum number of CREDIT to stake
-    uint256 public constant MIN_STAKE = 1e18;
+    uint256 public minStake ;

 constructor(
        address _core,
        address _profitManager,
        address _credit,
        address _guild,
        address _rlgm,
        uint256 _mintRatio,
        uint256 _rewardRatio
+     uint256 _minStake
    ) CoreRef(_core) {
        profitManager = _profitManager;
        credit = _credit;
        guild = _guild;
        rlgm = _rlgm;
        mintRatio = _mintRatio;
        rewardRatio = _rewardRatio;
+     minStake = _minStake
    }

Assessed type

Context

[c4-pre-sort]

0xSorryNotSorry marked the issue as sufficient quality report

[c4-pre-sort]

0xSorryNotSorry marked the issue as primary issue

[eswak]

Acknowledging, there would be a bunch of constants through the protocol that need update, depending on the value of the pegToken and the network where we deploy. I'd flag this a QA.

[c4-sponsor]

eswak (sponsor) acknowledged

[c4-sponsor]

eswak marked the issue as disagree with severity

[c4-judge]

Trumpero changed the severity to QA (Quality Assurance)

[c4-judge]

Trumpero marked the issue as grade-a

[c4-judge]

Trumpero marked the issue as grade-b

[blckhv]

Hello, @Trumpero, thanks for judging this.
We would like to request reconsideration as it explicitly highlights a flaw in the system when utilizing more expensive credit tokens such as wBTC and ETH and there is no way to modify the minimum stake amount required.

While it costs $1 to start staking in gUSDC gauge, for wBTC (8 decimals) 1e10 tokens will be needed to meet the minimum requirement for participating, and for ETH (18 decimals) 1 token which will cost around $2000.

As seen this will limit the amount of users willing to stake in such gauges.

There is a similar confirmed issue from the Moonwell contest: code-423n4/2023-07-moonwell-findings#143

[NicolaMirchev]

I totally agree with @AydoanB.

Also in terms of C4 it is stated that code in the README should be audited/judged as if it were going to be uploaded that way, which would lead to major impact, because large amount of the participants won't have the capital for the minStake, or don't wan't to use such big amount. Also it is confirmed several times that ETH would be supported on launch, which confirms the severity and validity of the issue.

[Trumpero]

This kind of issue will be judged by the individual opinion of the judge. In my opinion, the severity of this issue is pretty low since it doesn't have any impact to be considered medium:

• No funds are at risk.
• No invariants are broken.
• Core functionalities won't be bricked or impacted.

Its maximum impact is that users can't stake below 1 WBTC in the WBTC market, which I consider a business issue since 1 WBTC is a possible amount. The issue from the Moonwell contest mentions the impact that deployment will fail, so I believe it's not similar.