Ethereum Credit Guild - mojito_auditor's results

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/8f439544b8a634e8f3c3db718416a39752cd471e/src/loan/LendingTerm.sol#L323-L330

Vulnerability details

Impact

The issue is apparent upon examining the code snippet below. It should return the minimum of the three values, but it doesn't.

// return min(creditMinterBuffer, hardCap, debtCeiling) 
// @audit this is not min of 3
if (creditMinterBuffer < _debtCeiling) {
    return creditMinterBuffer;
}
if (_hardCap < _debtCeiling) {
    return _hardCap;
}
return _debtCeiling;

Proof of Concept

Consider these parameters: creditMinterBuffer = 10, _debtCeiling = 20, _hardCap = 9.

The code breaks right after the first if statement, returning creditMinterBuffer = 10 when the minimum of the three should be 9 (_hardCap).

Tools Used

Manual Review

Recommended Mitigation Steps

uint256 minValue = _debtCeiling;
if (creditMinterBuffer < minValue) {
    minValue = creditMinterBuffer;
}
if (_hardCap < minValue) {
    minValue = _hardCap;
}
return minValue;

Assessed type

Other

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/8f439544b8a634e8f3c3db718416a39752cd471e/src/loan/LendingTerm.sol#L279

Vulnerability details

Impact

The debtCeiling() function calculates the maximum amount of debt that can be issued by this term according to the current gauge allocations. It also takes an additional input parameter gaugeWeightDelta representing a hypothetical change in gauge weight.

However, gaugeWeightDelta is added to gaugeWeight, but not to totalWeight for the calculation. Consequently, the debt ceiling may not be computed accurately.

Proof of Concept

Consider the if-else block below. If gaugeWeight == totalWeight, it immediately returns min(_hardCap, creditMinterBuffer) without further calculations. This is based on the assumption that there is only one gauge. However, since totalWeight is not updated, this assumption may not hold true. For instance, if there are two gauges with weights [50, 50] (totalWeight = 100), and the first gauge increases by 50 to become [100, 50], the check will pass because totalWeight is not updated (it should be updated to 150 instead of 100).

function debtCeiling(
    int256 gaugeWeightDelta
) public view returns (uint256) {
    address _guildToken = refs.guildToken; // cached SLOAD
    uint256 gaugeWeight = GuildToken(_guildToken).getGaugeWeight(
        address(this)
    );
    gaugeWeight = uint256(int256(gaugeWeight) + gaugeWeightDelta);
    uint256 gaugeType = GuildToken(_guildToken).gaugeType(address(this));
    uint256 totalWeight = GuildToken(_guildToken).totalTypeWeight(
        gaugeType
    ); // @audit totalWeight is not added gaugeWeightDelta
    uint256 creditMinterBuffer = RateLimitedMinter(refs.creditMinter)
        .buffer();
    uint256 _hardCap = params.hardCap; // cached SLOAD
    if (gaugeWeight == 0) {
        return 0; // no gauge vote, 0 debt ceiling
    } else if (gaugeWeight == totalWeight) {
        // one gauge, unlimited debt ceiling
        // returns min(hardCap, creditMinterBuffer)
        return
            _hardCap < creditMinterBuffer ? _hardCap : creditMinterBuffer;
    }
    ...
}

Tools Used

Manual Review

Recommended Mitigation Steps

Add gaugeWeightDelta to totalWeight.

Assessed type

Other