Digital garden

code4rena-2024-03-dittoeth-m04

3 min read

code4rena-2024-03-dittoeth-m04

[M-04] transferShortRecord: Can transfer a newly created ShortRecord using a previously minted NFT

보고서

Summary

숏을 NFT화 한 뒤 연동된 숏을 닫더라도 NFT가 소각되지 않고, 숏 ID를 재활용했을 때 과거 숏에 매핑된 NFT를 이용하여 동일 ID의 새로운 숏을 이동할 수 있다.

Keyword

logic flaw, nft, erc721

Vulnerability

숏은 NFT로 만들어 이동할 수 있다. 그런데 숏이 닫힌 후에도 숏 NFT는 소각되지 않는다. 숏 ID는 숏이 닫히면 재활용 리스트에 들어가고, 새로운 숏을 생성할 때 재활용 리스트에 있는 ID를 먼저 사용한다.

그리고 NFT를 이동할 때, NFT에 연동된 숏 ID가 실제로 NFT를 만들 당시 있던 숏인지를 확인하지 않는다. 따라서 동일한 ID를 가진 과거 숏에 연동된 NFT를 이동하면 새로 생성된 숏을 이동시킬 수 있다. 과거에 발행한 NFT를 외부 컨트랙트 등에 approve 해둔 경우 의도하지 않게 미래에 생성된, 동일 ID를 가진 숏이 이동할 수 있다.

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]; // no check for tokenId
    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;
}

Impact

과거 숏과 연동되었던 NFT를 통해 동일 ID를 가진 미래의 숏이 이동할 수 있다. NFT가 잘못 매핑된다.

Mitigation

숏과 연동된 NFT id가 일치하는지를 확인하여 미래의 숏이 과거의 NFT로 인해 잘못 옮겨지는 것을 방지한다.

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();
+   if (short.tokenId != tokenId) revert Errors.NotValidNFT();
 
    // @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;
}

tags: bughunting, dittoeth, smart contract, solidity, logic flaw, nft, erc721, solo issue, severity medium

Graph View