hats-finance / Intuition-0x538dbadc50cc87b281cd655f1edbc6ebda02a66a

The smart contracts of the Intuition protocol v1.
https://intuition.systems
Other
0 stars 1 forks source link

Exit fees are accounted towards the total vault assets when they're updated during redeems #19

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

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

Github username: -- Twitter username: iamandreiski Submission hash (on-chain): 0xc1de819c997a15e9cee77e504a3ca5cddd182703532cabeac1882dac65fd012d Severity: medium

Description: Description/

When users deposit or redeem either Atom or Triple, there can be entry or exit fees. In both cases the fees are accounted towards the vault assets. Since entry fees are charged only when the vault is empty, in this case we'll focus on the exit fees.

When a user redeems, the getRedeemAssetsAndFees() function determines the amount of fees that a user will pay. The exit fee amount is never subtracted from the total vault assets and it will remain as part of it, even though shares which correlate to its part were burned during the redemption.

Attack Scenario/

When a user decides to redeem Atom or Triple, they will choose the respective function, which will subsequently call the _redeem() function:


 (, uint256 assetsForReceiver, uint256 protocolFee, uint256 exitFee) = getRedeemAssetsAndFees(shares, id);

        // set vault totals (assets and shares)
        _setVaultTotals(
            id,
            vaults[id].totalAssets - (assetsForReceiver + protocolFee), // totalAssetsDelta
            vaults[id].totalShares - shares // totalSharesDelta
        );

When getRedeemAssetsAndFees() is examined:

 else if (remainingShares == generalConfig.minShare) {
            exitFee = 0;
            protocolFee = protocolFeeAmount(assetsForReceiverBeforeFees, id);
        } else {
            protocolFee = protocolFeeAmount(assetsForReceiverBeforeFees, id);
            uint256 assetsForReceiverAfterprotocolFee = assetsForReceiverBeforeFees - protocolFee;
            exitFee = exitFeeAmount(assetsForReceiverAfterprotocolFee, id);
        }

        uint256 totalUserAssets = assetsForReceiverBeforeFees;
        uint256 assetsForReceiver = assetsForReceiverBeforeFees - exitFee - protocolFee;

        return (totalUserAssets, assetsForReceiver, protocolFee, exitFee);

When it comes to assetsForReceiver which the _setVaultTotals uses to account for vault assets, it's the asset amount without the exit and the protocol fee. When _setVaultTotals() is invoked to update the totalAsset values, it accounts for the assetsForReceiver and the protocol fee, but the exitFee will remain as part of the totalAssets in the vault, even though corresponding shares for it were burned when later burn was invoked:

 // burn shares, then transfer assets to receiver
        _burn(owner, id, shares);

Attachments

  1. Proof of Concept (PoC) File

  2. Revised Code File (Optional)

iamandreiski commented 3 months ago

Seems like a mistake on my part as it is expected behavior, just saw it in the docs, the issue can be closed.

mihailo-maksa commented 3 months ago

Upon review, we confirm that the behavior described in the report is indeed intended and documented. Here are the key points:

  1. Intended Design: The exit fee is accounted for correctly in the vault's total assets as designed. This approach ensures that the protocol maintains accurate records of assets and shares.
  2. Acknowledgement by Reporter: The report author has acknowledged the intended behavior and retracted the issue.

In conclusion, the system works as intended, and this issue can be closed.