hats-finance / Inverter-Network-0xe47e52c4fea05e555920f1dcdcc6fb8eca103eeb

Fork of the Inverter Smart Contracts Repository
GNU Lesser General Public License v3.0
0 stars 3 forks source link

FM_Rebasing_v1 and ElasticReceiptTokenBase_v1 have different "caps" on deposit #47

Open hats-bug-reporter[bot] opened 5 months ago

hats-bug-reporter[bot] commented 5 months ago

Github username: -- Twitter username: @EgisSec Submission hash (on-chain): 0x165b6846a51cb69f358cb6571844ca29c53d0a10b531db08670632d04a443730 Severity: low

Description: Description\ If we take a look at _deposit in FM_Rebasing_v1

function _deposit(address from, address to, uint amount) internal {
        // Depositing from itself with its own balance would mint tokens without increasing underlying balance.
        if (from == address(this)) {
            revert Module__FundingManager__CannotSelfDeposit();
        }

        if ((amount + token().balanceOf(address(this))) > DEPOSIT_CAP) {
            revert Module__FundingManager__DepositCapReached();
        }

        _mint(to, amount);

        token().safeTransferFrom(from, address(this), amount);

        emit Deposit(from, to, amount);
    }

You can see that there is a check for DEPOSIT_CAP, which is hardcoded to 100_000_000e18.

When _deposit is called, _mint will be called in ElasticReceiptTokenBase_v1

function _mint(address to, uint tokens)
        internal
        validRecipient(to)
        validAmount(tokens)
        onAfterRebase
    {
        // Do not mint more than allowed.
        if (_totalTokenSupply + tokens > MAX_SUPPLY) {
            revert MaxSupplyReached();
        }

        // Get amount of bits to mint and new total amount of active bits.
        uint bitsNeeded = _tokensToBits(tokens);
        uint newActiveBits = _activeBits() + bitsNeeded;

        // Increase total token supply and adjust conversion rate only if no
        // conversion rate defined yet. Otherwise the conversion rate should
        // not change as the downstream contract is assumed to increase the
        // supply target by precisely the amount of tokens minted.
        _totalTokenSupply += tokens;
        if (_bitsPerToken == 0) {
            _bitsPerToken = newActiveBits / _totalTokenSupply;
        }

        // Notify about new rebase.
        _epoch++;
        emit Rebase(_epoch, _totalTokenSupply);

        // Transfer newly minted bits from zero address.
        _transfer(address(0), to, tokens, bitsNeeded);
    }

The function also has a cap on the amount of tokens that the contract can hold.

The issue is that MAX_SUPPLY is 1_000_000_000e18, which has 1 more 0 than DEPOSIT_CAP.

Considering both variables check the same thing, they should be in sync or one of them should be removed.

Attack Scenario\

Attachments

  1. Proof of Concept (PoC) File

  2. Revised Code File (Optional)

Sync both variables or remove one of them, as they are effectively checking the same thing.

PlamenTSV commented 5 months ago

The first deposit cap inside _deposit protects us from ever reaching the higher MAX_SUPPLY. I see it as intended if the protocol has in mind other ways to mint or distribute Elastic tokens, different than deposits.