code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-11-paraspace/blob/c6820a279c64a299a783955749fdc977de8f0449/paraspace-core/contracts/protocol/libraries/logic/FlashClaimLogic.sol#L28

Vulnerability details

Background

When a Moonbird NFT is nested, the transfer function is disabled. Thus, the usual ERC721's transfer, transferFrom and safeTransferFrom function will not work for a Nested Moonbird NFT.

The following is extracted directly from ParaSpace's Audit Technical documentation.

Specifically, MoonBirds has a nesting functionality which disables the the function safeTransferFrom . additionally, it has a nesting toggle and nesting information that holders would like to have access to after supplying the token to Paraspace. In order to support the transferring of the token while nesting the “owner (not approved operators)” of the token can call a custom function called safeTransferWhileNesting

The flash claim feature relies on the ERC721's transfer, transferFrom, and safeTransferFrom functions. Thus, if the users attempt to perform a flash claim against his Nested MoonBird NFT, the function will revert.

Proof of Concept

The flash claim feature relies on the FlashClaimLogic.executeFlashClaim function. Within the FlashClaimLogic.executeFlashClaim function, it relies on ERC721's safeTransferFrom function to move underlying NFT forward to and backward from receiver contract. Since Nested MoonBird NFT disable the usual ERC721's transfer functionaility, the function will revert.

https://github.com/code-423n4/2022-11-paraspace/blob/c6820a279c64a299a783955749fdc977de8f0449/paraspace-core/contracts/protocol/libraries/logic/FlashClaimLogic.sol#L28

File: FlashClaimLogic.sol
28:     function executeFlashClaim(
29:         DataTypes.PoolStorage storage ps,
30:         DataTypes.ExecuteFlashClaimParams memory params
31:     ) external {
32:         DataTypes.ReserveData storage reserve = ps._reserves[params.nftAsset];
33:         address nTokenAddress = reserve.xTokenAddress;
34:         ValidationLogic.validateFlashClaim(ps, params);
35: 
36:         uint256 i;
37:         // step 1: moving underlying asset forward to receiver contract
38:         for (i = 0; i < params.nftTokenIds.length; i++) {
39:             INToken(nTokenAddress).transferUnderlyingTo(
40:                 params.receiverAddress,
41:                 params.nftTokenIds[i]
42:             );
43:         }
44: 
45:         // step 2: execute receiver contract, doing something like airdrop
46:         require(
47:             IFlashClaimReceiver(params.receiverAddress).executeOperation(
48:                 params.nftAsset,
49:                 params.nftTokenIds,
50:                 params.params
51:             ),
52:             Errors.INVALID_FLASH_CLAIM_RECEIVER
53:         );
54: 
55:         // step 3: moving underlying asset backward from receiver contract
56:         for (i = 0; i < params.nftTokenIds.length; i++) {
57:             IERC721(params.nftAsset).safeTransferFrom(
58:                 params.receiverAddress,
59:                 nTokenAddress,
60:                 params.nftTokenIds[i]
61:             );
62: 
63:             emit FlashClaim(
64:                 params.receiverAddress,
65:                 msg.sender,
66:                 params.nftAsset,
67:                 params.nftTokenIds[i]
68:             );
69:         }
..SNIP..
90:     }

Impact

Users will not be able to use the flash-claim feature to claim their nested MoonBird NFT. If the users need to flash-claim their nested MoonBird NFT to claim any rewards or airdrop, they will not be able to do so.

Recommended Mitigation Steps

Considering implement a seperate flash-claim feature for nested MoonBird NFT that depends on safeTransferWhileNesting function instead of safeTransferFrom function

c4-judge commented 1 year ago

dmvt changed the severity to QA (Quality Assurance)