c4-bot-1 commented 7 months ago

Lines of code

https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/facets/ShortRecordFacet.sol#L40-L70

Vulnerability details

Impact

Shorter will lose so far gained yield if chooses to increase the collateral of his position. ShortRecord.updatedAt and ShortRecord.dethYieldRate are updated but the yield is not given to the user.

Proof of Concept

You can place the following 2 tests at the bottom of the file in BidOrders.t.sol.

Example of a flow with increased collateral and the lost yield:

function test_increaseCollateral_lostYield() public {
        address seller = makeAddr("seller");
        address buyer = makeAddr("buyer");

        // 1. Create Short Record
        fundLimitShortOpt(DEFAULT_PRICE, DEFAULT_AMOUNT, seller);
        STypes.Order[] memory shorts = diamond.getShorts(asset);
        uint8 shortRecordId = shorts[0].shortRecordId;

        fundLimitBidOpt(DEFAULT_PRICE, DEFAULT_AMOUNT, buyer);

        STypes.ShortRecord memory shortRecordAfterMatch = diamond.getShortRecord(asset, seller, shortRecordId);

        // 2. Pass time and mimic rate change
        skip(3600); // 1 hour
        diamond.setDethYieldRate(vault, 1e15);

        // 3. Increase collateral with 1 eth
        uint88 increaseCollateralAmount = 1 ether;
        depositEth(seller, increaseCollateralAmount);
        vm.prank(seller);
        diamond.increaseCollateral(asset, shortRecordId, increaseCollateralAmount);

        // 4. Exit Short Record
        depositUsd(seller, DEFAULT_AMOUNT);
        vm.prank(seller);
        diamond.exitShortErcEscrowed(asset, shortRecordId, DEFAULT_AMOUNT, 0);

        STypes.VaultUser memory userVaultAfterExit = diamond.getVaultUserStruct(1, seller);

        // no yield was gained
        assert(userVaultAfterExit.ethEscrowed == increaseCollateralAmount + shortRecordAfterMatch.collateral);
    }

Example of a flow without increased collateral and gained yield:

function test_gainedYield() public {
        address seller = makeAddr("seller");
        address buyer = makeAddr("buyer");

        // 1. Create Short Record
        fundLimitShortOpt(DEFAULT_PRICE, DEFAULT_AMOUNT, seller);
        STypes.Order[] memory shorts = diamond.getShorts(asset);
        uint8 shortRecordId = shorts[0].shortRecordId;

        fundLimitBidOpt(DEFAULT_PRICE, DEFAULT_AMOUNT, buyer);

        STypes.ShortRecord memory shortRecordAfterMatch = diamond.getShortRecord(asset, seller, shortRecordId);

        // 2. Pass time and mimic rate change
        skip(3600); // 1 hour
        diamond.setDethYieldRate(vault, 1e15);

        // 3. Exit Short Record
        depositUsd(seller, DEFAULT_AMOUNT);
        vm.prank(seller);
        diamond.exitShortErcEscrowed(asset, shortRecordId, DEFAULT_AMOUNT, 0);

        STypes.VaultUser memory userVaultAfterExit = diamond.getVaultUserStruct(1, seller);

        // yield was gained
        assert(userVaultAfterExit.ethEscrowed != shortRecordAfterMatch.collateral);
        assert(userVaultAfterExit.ethEscrowed > shortRecordAfterMatch.collateral);
    }