`MAX_SUPPLY` not checked when `USDA._mint` is called

[code423n4]

Lines of code

https://github.com/code-423n4/2023-07-amphora/blob/main/core/solidity/contracts/core/USDA.sol#L167-L178

Vulnerability details

Impact

In the USDA contract the variable representing the total supply _totalSupply has a maximum value set in the UFragments contract as MAX_SUPPLY = 2 ** 128 - 1, but when the function USDA._mint is called (by one of the functions : mint, _deposit, vaultControllerMint) it doesn't check that the new _totalSupply is indeed less than MAX_SUPPLY before minting new tokens, hence the total supply will surpass the limit set at some point in the future which can results in some problems for the protocol.

Proof of Concept

The issue is occurring in the _mint function below :

function _mint(address _target, uint256 _amount) internal {
    uint256 __gonsPerFragment = _gonsPerFragment;
    // the gonbalances of the sender is in gons, therefore we must multiply the deposit amount, which is in fragments, by gonsperfragment
    _gonBalances[_target] += _amount * __gonsPerFragment;

    // @audit MAX_SUPPLY not checked before incrementing
    // total supply is in fragments, and so we add amount
    _totalSupply += _amount;
    // and totalgons of course is in gons, and so we multiply amount by gonsperfragment to get the amount of gons we must add to totalGons
    _totalGons += _amount * __gonsPerFragment;
    // emit both a mint and transfer event
    emit Transfer(address(0), _target, _amount);
    emit Mint(_target, _amount);
}

As you can see from the code above, when the _mint function is called it does increment the value of _totalSupply immediately without checking if it will surpass the maximum supply amount set in the UFragments contract :

// MAX_SUPPLY = maximum integer < (sqrt(4*_totalGons + 1) - 1) / 2
uint256 public MAX_SUPPLY; // = type(uint128).max; // (2^128) - 1

MAX_SUPPLY = 2 ** 128 - 1;

In the USDA contract, both internal functions _mint and _donation are used to increment the value of _totalSupply when they are called by others public/external functions. The function _donation does perform a check to ensure that _totalSupply is always less than MAX_SUPPLY, but as we saw above the _mint function does not do the same, this will for sure cause the _totalSupply to surpass the maximum value at some point in the future which can in return result in some problems for the protocol as the logic parameters values will be out of range.

Tools Used

Manual review

Recommended Mitigation Steps

To fix this issue, i recommend to add a check in the _mint function to ensure that the value of _totalSupply is always less than the maximum value.

The function should be modified as follows :

function _mint(address _target, uint256 _amount) internal {
    uint256 __gonsPerFragment = _gonsPerFragment;
    // the gonbalances of the sender is in gons, therefore we must multiply the deposit amount, which is in fragments, by gonsperfragment
    _gonBalances[_target] += _amount * __gonsPerFragment;

    // total supply is in fragments, and so we add amount
    // @audit check if less than max
    if (_totalSupply + _amount > MAX_SUPPLY) revert();
    _totalSupply += _amount;
    // and totalgons of course is in gons, and so we multiply amount by gonsperfragment to get the amount of gons we must add to totalGons
    _totalGons += _amount * __gonsPerFragment;
    // emit both a mint and transfer event
    emit Transfer(address(0), _target, _amount);
    emit Mint(_target, _amount);
}

Assessed type

Error

[c4-pre-sort]

minhquanym marked the issue as duplicate of #287

[c4-judge]

dmvt marked the issue as unsatisfactory: Overinflated severity