c4-bot-7 commented 3 months ago

Lines of code

https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/libraries/LibSRUtil.sol#L124-L150

Vulnerability details

Proof of Concept

Take a look at https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/libraries/LibSRUtil.sol#L124-L150

    function transferShortRecord(address from, address to, uint40 tokenId) internal {
        AppStorage storage s = appStorage();

        STypes.NFT storage nft = s.nftMapping[tokenId];
        address asset = s.assetMapping[nft.assetId];
        STypes.ShortRecord storage short = s.shortRecords[asset][from][nft.shortRecordId];
        if (short.status == SR.Closed) revert Errors.OriginalShortRecordCancelled();
        if (short.ercDebt == 0) revert Errors.OriginalShortRecordRedeemed();

        // @dev shortOrderId is already validated in mintNFT
        if (short.status == SR.PartialFill) {
            LibOrders.cancelShort(asset, nft.shortOrderId);
        }

        short.tokenId = 0;
        LibShortRecord.deleteShortRecord(asset, from, nft.shortRecordId);
        LibBridgeRouter.transferBridgeCredit(asset, from, to, short.collateral);

        uint8 id = LibShortRecord.createShortRecord(
            asset, to, SR.FullyFilled, short.collateral, short.ercDebt, short.ercDebtRate, short.dethYieldRate, tokenId
        );

        nft.owner = to;
        nft.shortRecordId = id;
        nft.shortOrderId = 0;
    }

The transferShortRecord function in the provided code snippet allows for the transfer of short records as NFTs (ERC-721) between addresses. However, it introduces a vulnerability where any user can use this as a method to sidestep liquidations, this is cause while transferring the short records there are no checks that the position is currently above liquidatable grounds, i.e a user having a ShortRecord with low collateral ratio can just transfer it to another of their own address by front running aliquidation. Since the liquidator in this case has to pass both the shorter and the ids to specify the short, any attempt to liquidate the short will fail.

Impact

This leads to bad collateral ratio for the system since liquidations can be easily sidestepped.

Recommended Mitigation Steps

While transferring a short record, check if the collateral ratio is below the primary liquidation threshold and then revert on instances like this

Assessed type

Context

c4-pre-sort commented 3 months ago

raymondfam marked the issue as sufficient quality report

c4-pre-sort commented 3 months ago

raymondfam marked the issue as duplicate of #111

raymondfam commented 3 months ago

See #111.

c4-judge commented 3 months ago

hansfriese marked the issue as unsatisfactory: Out of scope

code-423n4 / 2024-03-dittoeth-findings