hats-finance / Tokemak-0x4a2d708ea6b0c04186ecb774cfad1e50fb5efc0b

0 stars 0 forks source link

LMPStrategy.sol#getDestinationTrimAmount() - Trimming applied incorrectly #8

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

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

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

Description: Description\ When trimming logic is applied we have 2 scenarios.


 function getDestinationTrimAmount(IDestinationVault dest) internal returns (uint256) {
        uint256 discountThresholdOne = 3e5; // 3% 1e7 precision, discount required to consider trimming
        uint256 discountDaysThreshold = 7; // number of last 10 days that it was >= discountThreshold
        uint256 discountThresholdTwo = 5e5; // 5% 1e7 precision, discount required to completely exit

        // this is always the out destination and guaranteed not to be the LMPVault idle asset
        IDexLSTStats.DexLSTStatsData memory stats = dest.getStats().current();

        ILSTStats.LSTStatsData[] memory lstStats = stats.lstStatsData;
        uint256 numLsts = lstStats.length;

        uint256 minTrim = 1e18; // 100% -- no trim required
        for (uint256 i = 0; i < numLsts; ++i) {
            ILSTStats.LSTStatsData memory targetLst = lstStats[i];
            (uint256 numViolationsOne, uint256 numViolationsTwo) =
                getDiscountAboveThreshold(targetLst.discountHistory, discountThresholdOne, discountThresholdTwo);

            if (targetLst.discount >= int256(discountThresholdTwo * 1e11) && numViolationsTwo >= discountDaysThreshold)
            {
                // this is the worst possible trim, so we can exit without checking other LSTs
                return 0;
            }

            // discountThreshold is 1e7 precision for the discount history, but here it is compared to a 1e18, so pad it
            if (targetLst.discount >= int256(discountThresholdOne * 1e11) && numViolationsOne >= discountDaysThreshold)
            {
                minTrim = minTrim.min(1e17); // 10%
            }
        }

        return minTrim;
    }

We loop through lstStats and calculate if any of the discounts have broken their constraints.

In the second case targetLst.discount >= int256(discountThresholdOne * 1e11) && numViolationsOne >= discountDaysThreshold

We reduce minTrim by 10%, but only the first time as the logic uses .min

minTrim = minTrim.min(1e17)

Since minTrim = 1e18 and if we call minTrim.min(1e17) once, minTrim is now 1e17, as 1e17 < 1e18.

But if we again hit the second if and go in it, the 10% won't apply, as we are comparing 1e17 < 1e17, so minTrim will stay at 1e17.

Since this happens in a loop, we can enter targetLst.discount >= int256(discountThresholdOne * 1e11) && numViolationsOne >= discountDaysThreshold multiple times, but apply the 10% only once.

Attack Scenario\

Attachments

  1. Proof of Concept (PoC) File

  2. Revised Code File (Optional)

codenutt commented 5 months ago

The trim applies to the position as a whole, not the constituent tokens so it is appropriate to only apply the 10% 1 time.