code-423n4 / 2024-06-size-findings

0 stars 0 forks source link

Users can avoid paying fragmentation fees #394

Closed howlbot-integration[bot] closed 2 months ago

howlbot-integration[bot] commented 2 months ago

Lines of code

https://github.com/code-423n4/2024-06-size/blob/8850e25fb088898e9cf86f9be1c401ad155bea86/src/libraries/actions/Compensate.sol#L31

Vulnerability details

Impact

Users can avoid paying fragmentation fees

Proof of Concept

Users can use credit positions to take out loans. If they take an amount that splits their credit position, then they owe a fragmentation fee. The issue is that this can be avoided as users can use compensate() to split credit positions without a fee. They can split the position in such a way that whenever they input the amount they want to lend/borrow, that amount would be equal to the credit of the position and thus, they would avoid paying fragmentation fees.

For the POC, create an Event log in BuyCreditMarket.sol and add 2 events like shown below (on the 2 last lines):

if (params.exactAmountIn) {
            cashAmountIn = params.amount;
            (creditAmountOut, fees) = state.getCreditAmountOut({
                cashAmountIn: cashAmountIn,
                maxCashAmountIn: params.creditPositionId == RESERVED_ID
                    ? cashAmountIn
                    : Math.mulDivUp(creditPosition.credit, PERCENT, PERCENT + ratePerTenor),
                maxCredit: params.creditPositionId == RESERVED_ID
                    ? Math.mulDivDown(cashAmountIn, PERCENT + ratePerTenor, PERCENT)
                    : creditPosition.credit,
                ratePerTenor: ratePerTenor,
                tenor: tenor
            });
        } else {
            creditAmountOut = params.amount;
            (cashAmountIn, fees) = state.getCashAmountIn({
                creditAmountOut: creditAmountOut,
                maxCredit: params.creditPositionId == RESERVED_ID ? creditAmountOut : creditPosition.credit,
                ratePerTenor: ratePerTenor,
                tenor: tenor
            });
            emit Log(creditAmountOut);
            emit Log(params.creditPositionId == RESERVED_ID ? creditAmountOut : creditPosition.credit);
        }

If these values are equal, then that means the position did not get fragmented and the user doesn't have to pay fragmentation fees. If they are not, the user has to pay fragmentation fees. Paste the following POC into Compensate.t.sol (use -vvvv to see the events and you can also comment out the lines where Alice is compensating to see the difference in events):

function testUserCanAvoidPayingFragmentationFees() public {
        _setPrice(1e18);

        _deposit(bob, usdc, 100e6);
        _deposit(alice, weth, 150e18);
        _deposit(alice, usdc, 100e6);

        _buyCreditLimit(bob, type(uint256).max, YieldCurveHelper.pointCurve(1 hours, 0));
        uint256 debtPositionId = _sellCreditMarket(alice, bob, RESERVED_ID, 100e6, 1 hours, true);
        uint256 creditPositionId = size.getCreditPositionIdsByDebtPositionId(debtPositionId)[0];

        _sellCreditLimit(bob, YieldCurveHelper.pointCurve(1 hours, 0));

        vm.prank(alice);
        size.compensate(CompensateParams({creditPositionWithDebtToRepayId: creditPositionId, creditPositionToCompensateId: RESERVED_ID, amount: 10e6}));

        _buyCreditMarket(alice, bob, creditPositionId, 90e6, 1 hours, false);
    }

Tools Used

Manual Review

Recommended Mitigation Steps

It is not trivial to fix this if you want to keep the code the same. Implementing a fee upon compensating is a possible fix.

Assessed type

Other

c4-judge commented 2 months ago

hansfriese changed the severity to 2 (Med Risk)

c4-judge commented 2 months ago

hansfriese marked the issue as duplicate of #10

c4-judge commented 2 months ago

hansfriese marked the issue as satisfactory