Users can pay more than expected fee due to lack of slippage control on tokenization fee when depositing IBT tokens

[hats-bug-reporter]

Github username: @0xSwahili
Twitter username: --
Submission hash (on-chain): 0x468bd6737182dd0a8ad12826c0130b1894c9a1c18627568f8e26abd58f1e68c2
Severity: high

Description:
Description
When depositing IBT token to the PrincipleToken contract, users pay a tokenization fee:

 function _depositIBT(
        uint256 _ibts,
        address _ptReceiver,
        address _ytReceiver
    ) internal notExpired returns (uint256 shares) {
        updateYield(_ytReceiver);
        uint256 tokenizationFee = _ibts._computeTokenizationFee(address(this), registry);
        _updateFees(tokenizationFee);
        shares = _convertIBTsToShares(_ibts - tokenizationFee, false);
        if (shares == 0) {
            revert RateError();
        }
        _mint(_ptReceiver, shares);
        emit Mint(msg.sender, _ptReceiver, shares);
        IYieldToken(yt).mint(_ytReceiver, shares);
    }

The vulnerability arises from the fact that this fee can set at any time by the protocol authority account :

function setTokenizationFee(uint256 _tokenizationFee) external override restricted {
        if (_tokenizationFee > MAX_TOKENIZATION_FEE) {
            revert FeeGreaterThanMaxValue();
        }
        emit TokenizationFeeChange(tokenizationFee, _tokenizationFee);
        tokenizationFee = _tokenizationFee;
    }

If the fee goes up, users can end-up paying higher fees than expected.

Attack Scenario
Consider the following scenario:

1. User A wishes to make several deposits from his contract.
2. He therefore calculates the required amounts plus fee to facilitate the deposits.
3. Finally he sends the deposit transactions.
4. Meanwhile, the protocol authority wants to increases the fee by 5%. He therefore calls setTokenizationFee, but with a higher gas price than User A.
5. When the transactions are processed, the validator chooses to prioritize the protocol authority's transaction ahead of user A's calls .

Due to the fee increment, the protocol user will pay more than expected fee causing some transactions to fail with an ERC20InsufficientAllowance error.

This scenario is even more likely to happen if the protocol admin chooses to send his transaction via MEV or during periods of high gas price volatility that can motivate the protocol authority to send his transaction with high gas price. Even if a TimeLock contract is used to update such fee, users can still end up with suprise fee changes since that requires users to keep up with the relevant updating contracts.

Attachments

1. Proof of Concept (PoC) File

This POC shows that a deposit transaction can fail due to insufficient token approval:

function testInsufficientAllowance() public {
        uint256 amountToDeposit = 1e18;
        underlying.approve(address(principalToken), amountToDeposit - 500);
        bytes memory revertData = abi.encodeWithSelector(
            bytes4(keccak256("ERC20InsufficientAllowance(address,uint256,uint256)")),
            address(principalToken),
            amountToDeposit - 500,
            amountToDeposit
        );
        vm.expectRevert(revertData);
        principalToken.deposit(amountToDeposit, MOCK_ADDR_1);
    }

2. Revised Code File (Optional)

• Consider introducing a maxFee parameter for the deposit and depositIBT functions and check that the value is non-zero

[yanisepfl]

Hello,

We classified this issue as Invalid because:

• We assume the protocol authority (DAO) not to be malicious.
• The PoC only shows how insufficient allowance work.

Thanks

[0xSwahili]

Hello,
The report does not say that the DAO needs to be malicious for the user to pay excess fee. There is a very real possibility for this scenario to happen since both parties are well motivated to act in their own interest even unaware of the other's actions. The DAO can send the update as part of other transactions. Maybe I can provide a better POC...

[0xSwahili]

ie the user is making a deposit, the DAO is updating the tokenization fee and the validator chooses to prioritize the DAO transactions. In this scenario, the user will definitely pay unexpected fee. All these conditions are very likely to happen.

[0xSwahili]

Can I proceed to provide a better POC ?