Staking even large amounts of CREDIT in SurplusGuildMinter is blocked by the requirement that the amount exceed MIN_STAKE

[c4-bot-9]

Lines of code

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

Vulnerability details

Impact

SurplusGuiltMinter.sol allows users to stake their CREDIT tokens to get GUILD tokens that can be used to vote for lending term gauges. This staking is performed by calling stake. The contract has a basic concept of MIN_STAKE to prevent people from staking dust amounts. The stake function requires that the amount to stake be larger than or equal to MIN_STAKE. The problem is MIN_STAKE is set to 1e18 but the stake function requires that amount exceed MIN_STAKE (not MIN_STAKE / 1e18). Therefore, any attempt to stake even a very large amount of tokens (like 1,000,000,000 which is probably more than any one user will have) will fail since you must stake 1,000,000,000,000,000,000+ tokens to pass the check.

Therefore, this is a complete denial of service to stake. See test below.

Proof of Concept

Here is the relevant code. The MIN_STAKE is set to 1e18 (presumably with the intent of preventing people from staking less than 1 token) link to code:

  uint256 public constant MIN_STAKE = 1e18;

Here is the stake function which requires that amount (to stake) exceeds or equals MIN_STAKE (not divided by 1e18) link to code:

function stake(address term, uint256 amount) external whenNotPaused {
        // apply pending rewards
        (uint256 lastGaugeLoss, UserStake memory userStake, ) = getRewards(
            msg.sender,
            term
        );

        require(
            lastGaugeLoss != block.timestamp,
            "SurplusGuildMinter: loss in block"
        );
        require(amount >= MIN_STAKE, "SurplusGuildMinter: min stake");

Run the following test to stake 1,000,000 CREDIT tokens - it will pass (ie, the attempt to stake will revert) though the person is staking a large number of tokens:

    function testStakeLargeAmount() public {
        vm.expectRevert("SurplusGuildMinter: min stake");
        sgm.stake(term, 1000000);
    }

This is what I get after running the test which shows that the revert due to failure to stake the minimum amount is the result:

Running 1 test for test/unit/loan/SurplusGuildMinter.t.sol:SurplusGuildMinterUnitTest
[PASS] testStakeLargeAmount() (gas: 39531)
Traces:
  [39531] SurplusGuildMinterUnitTest::testStakeLargeAmount() 
    ├─ [2569] profitManager::termSurplusBuffer(term: [0x1d1499e622D69689cdf9004d05Ec547d650Ff211]) [staticcall]
    │   └─ ← 0
    ├─ [2593] guild::balanceOf(sgm: [0xa0Cb889707d426A7A386870A03bc70d1b0697598]) [staticcall]
    │   └─ ← 0
    ├─ [2597] guild::getGaugeWeight(term: [0x1d1499e622D69689cdf9004d05Ec547d650Ff211]) [staticcall]
    │   └─ ← 50000000000000000000 [5e19]
    ├─ [0] VM::expectRevert(SurplusGuildMinter: min stake) 
    │   └─ ← ()
    ├─ [10585] sgm::stake(term: [0x1d1499e622D69689cdf9004d05Ec547d650Ff211], 1000000 [1e6]) 
    │   ├─ [2619] guild::lastGaugeLoss(term: [0x1d1499e622D69689cdf9004d05Ec547d650Ff211]) [staticcall]
    │   │   └─ ← 0
    │   └─ ← "SurplusGuildMinter: min stake"
    └─ ← ()
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 7.74ms

Tools Used

Foundry
Manual review

Recommended Mitigation Steps

Any time MIN_STAKE is used, it should be divided by 1e18.

Assessed type

DoS

[c4-pre-sort]

0xSorryNotSorry marked the issue as sufficient quality report

[c4-pre-sort]

0xSorryNotSorry marked the issue as duplicate of #597

[c4-judge]

Trumpero marked the issue as not a duplicate

[c4-judge]

Trumpero marked the issue as unsatisfactory:
Invalid

[Trumpero]

1,000,000 < 1e18 so it reverted correctly