ParaSpace contest - xiaoming90's results

Lines of code

https://github.com/code-423n4/2022-11-paraspace/blob/c01a980e5d6e15b2993b912c3569ed8b5236ff33/paraspace-core/contracts/ui/WPunkGateway.sol#L129

Vulnerability details

Background

History Of Punk NFT

Punk NFT was developed many years back before ERC721 standard was established. Thus, they took inspiration from the ERC20 standard and removed the transfer and transferFrom functions to achieve the "Non-Fungible" characteristic of their NFT "token".

Back then, there was also no approve or setApprovalForAll function since ERC721 standard had not been established yet. Thus, there is no way to grant allowance to the marketplace to spend or manage the punk NFTs on behalf of the users. As such, a workaround is needed, and the workaround is to use Punk.offerPunkForSaleToAddress, Punk.offerPunkForSale, and Punk.buyPunk to achieve the same result as allowance/approval in ERC721.

How it works is that the seller will call offerPunkForSaleToAddress to grant allowance to the marketplace/operator, and the marketplace/operator will call buyPunk to transfer the NFT from the seller.

This workaround is used within the WPunkGateway.acceptBidWithCredit function.

Proof of Concept

Following is the specification of the CryptoPunksMarket.offerPunkForSaleToAddress function.

https://github.com/code-423n4/2022-11-paraspace/blob/c6820a279c64a299a783955749fdc977de8f0449/paraspace-core/contracts/mocks/tokens/CryptoPunksMarket.sol#L210

File: CryptoPunksMarket.sol
210:     function offerPunkForSaleToAddress(
211:         uint256 punkIndex,
212:         uint256 minSalePriceInWei,
213:         address toAddress
214:     ) public {

The following gives an overview of how the WPunkGateway.acceptBidWithCredit function works under the hood. This is needed to understand the vulnerability.

1. Bob (maker) bids Alice's Punk NFT #2 for 1000 ETH. The 1000 ETH consists of 800 ETH payNowAmount and 200 ETH credit. In other words, Bob has to pay 800 ETH from his wallet and borrow 200 ETH from ParaSpace when the deal happens.

2. Suppose Alice (taker) wants to accept Bob's bid for her Punk NFT #2 (punkIndex=2). In that case, Alice will call the CryptoPunksMarket.offerPunkForSaleToAddress(2, 0, Punk_Gateway_Address) first.

3. Alice will call the WPunkGateway.acceptBidWithCredit function. The Punk Gateway will obtain/buy Alice's Punk NFT #2 from her wallet. Since the offer.minValue is zero, the Punk Gateway does not need to send any ETH for the transaction. Note that only the Punk Gateway can buy Alice's Punk NFT #2 because the offer.onlySellTo has been explicitly set to the Punk Gateway. Thus, there is no risk that other users can obtain Alice's Punk NFT #2 for zero wei.

4. The Punk Gateway will send the newly obtained Punk NFT #2 to WPunk Proxy to mint the Wrapped Punk NFT #2. Newly minted wrapped Punk NFT #2 will be returned to the Punk Gateway.

5. The Punk Gateway will send the wrapped Punk NFT #2 back to Alice's wallet.

6. The Pool.acceptBidWithCredit function in Line 148 below will be triggered, and the orders will be executed. Alice will receive 1000 ETH, and her Punk NFT #2 will the sent to ParaSpace.

7. ParaSpace will collateralize the Punk NFT #2 on Bob's behalf and mint respective nWPunk token to Bob that represents his claim to the Punk NFT #2 in ParaSpace.

https://github.com/code-423n4/2022-11-paraspace/blob/c01a980e5d6e15b2993b912c3569ed8b5236ff33/paraspace-core/contracts/ui/WPunkGateway.sol#L129

File: WPunkGateway.sol
129:     function acceptBidWithCredit(
130:         bytes32 marketplaceId,
131:         bytes calldata payload,
132:         DataTypes.Credit calldata credit,
133:         uint256[] calldata punkIndexes,
134:         uint16 referralCode
135:     ) external nonReentrant {
136:         for (uint256 i = 0; i < punkIndexes.length; i++) {
137:             Punk.buyPunk(punkIndexes[i]);
138:             Punk.transferPunk(proxy, punkIndexes[i]);
139:             // gatewayProxy is the sender of this function, not the original gateway
140:             WPunk.mint(punkIndexes[i]);
141: 
142:             IERC721(wpunk).safeTransferFrom(
143:                 address(this),
144:                 msg.sender,
145:                 punkIndexes[i]
146:             );
147:         }
148:         Pool.acceptBidWithCredit(
149:             marketplaceId,
150:             payload,
151:             credit,
152:             msg.sender,
153:             referralCode
154:         );
155:     }

However, there is a flaw in the WPunkGateway.acceptBidWithCredit function that allows anyone to steal Taker's Punk NFTs.

Assume that Alice calls the CryptoPunksMarket.offerPunkForSaleToAddress(2, 0, Punk_Gateway_Address) to "grant" the Punk Gateway the ability to obtain her Punk NFT #2 as described in the overview of the function above.

Next, Alice calls the WPunkGateway.acceptBidWithCredit function to accept the bid.

At this point, the sequence of execution in the mempool is as follows (First one will be executed first):

1. CryptoPunksMarket.offerPunkForSaleToAddress(2, 0, Punk_Gateway_Address) - Caller = Alice
2. WPunkGateway.acceptBidWithCredit(...punkIndexes=2...) - Caller Alice

Mallory (attacker) saw these transactions in the mempool and front-run Alice's second transaction. He calls the WPunkGateway.acceptBidWithCredit with the punkIndexes point to Punk NFT #2, and arbitrary bogus/non-relevant accept bid payloads (e.g. trading among his accounts) that does not cause a revert.

After the front-running attack, the sequence of execution in the mempool is as follows (First one will be executed first):

1. CryptoPunksMarket.offerPunkForSaleToAddress(2, 0, Punk_Gateway_Address) - Caller = Alice
2. WPunkGateway.acceptBidWithCredit(...punkIndexes=2...) - Caller Mallory (Front-Run)
3. WPunkGateway.acceptBidWithCredit(...punkIndexes=2...) - Caller Alice

Note that Line 142 of the WPunkGateway.acceptBidWithCredit will send the newly minted Wrapped Punk NFT #2 to the msg.sender. In this case, Mallory's WPunkGateway.acceptBidWithCredit transaction will be executed before Alice's one. Therefore, the caller is Mallory, and the Wrapped Punk NFT #2 will be sent to Mallory's wallet.

Alice's WPunkGateway.acceptBidWithCredit will subsequently revert.

Impact

Users' Punk NFTs will be stolen

Recommended Mitigation Steps

Update the WPunkGateway.acceptBidWithCredit to ensure that the newly minted wrapped Punk NFT is not blindly sent to the caller (msg.sender) because the caller might not be the owner/seller of the Punk NFT.

Instead, the newly minted wrapped Punk NFT should be sent back to the original owner/seller of the Punk NFT.

function acceptBidWithCredit(
	bytes32 marketplaceId,
	bytes calldata payload,
	DataTypes.Credit calldata credit,
	uint256[] calldata punkIndexes,
	uint16 referralCode
) external nonReentrant {
	for (uint256 i = 0; i < punkIndexes.length; i++) {
+		address originalOwner = Punk.punkIndexToAddress[punkIndexes[i]]; // Find the owner of the Punk NFT
+		
		Punk.buyPunk(punkIndexes[i]);
		Punk.transferPunk(proxy, punkIndexes[i]);
		// gatewayProxy is the sender of this function, not the original gateway
		WPunk.mint(punkIndexes[i]);

		IERC721(wpunk).safeTransferFrom(
			address(this),
-			msg.sender,
+			originalOwner
			punkIndexes[i]
		);
	}
	Pool.acceptBidWithCredit(
		marketplaceId,
		payload,
		credit,
		msg.sender,
		referralCode
	);
}

Lines of code

https://github.com/code-423n4/2022-11-paraspace/blob/c6820a279c64a299a783955749fdc977de8f0449/paraspace-core/contracts/misc/NFTFloorOracle.sol#L167

Vulnerability details

Proof of Concept

The NFTFloorOracle.removeFeeder function allows the caller to remove a feeder from the NFTFloorOracle. However, this function lacks of access control, and thus anyone can call this function to remove all feeders from the NFTFloorOracle.

https://github.com/code-423n4/2022-11-paraspace/blob/c6820a279c64a299a783955749fdc977de8f0449/paraspace-core/contracts/misc/NFTFloorOracle.sol#L167

File: NFTFloorOracle.sol
165:     /// @notice Allows owner to remove feeder.
166:     /// @param _feeder feeder to remove
167:     function removeFeeder(address _feeder)
168:         external
169:         onlyWhenFeederExisted(_feeder)
170:     {
171:         _removeFeeder(_feeder);
172:     }

The onlyWhenFeederExisted modifier only checks if the feeder exists. It does not check if the caller is the owner.

https://github.com/code-423n4/2022-11-paraspace/blob/c6820a279c64a299a783955749fdc977de8f0449/paraspace-core/contracts/misc/NFTFloorOracle.sol#L127

File: NFTFloorOracle.sol
127:     modifier onlyWhenFeederExisted(address _feeder) {
128:         require(_isFeederExisted(_feeder), "NFTOracle: feeder not existed");
129:         _;
130:     }

https://github.com/code-423n4/2022-11-paraspace/blob/c6820a279c64a299a783955749fdc977de8f0449/paraspace-core/contracts/misc/NFTFloorOracle.sol#L274

File: NFTFloorOracle.sol
274:     function _isFeederExisted(address _feeder) internal view returns (bool) {
275:         return feederPositionMap[_feeder].registered;
276:     }

Similarly, the _removeFeeder internal function only checks if the feeder exists. None of the code check if the caller is the owner.

https://github.com/code-423n4/2022-11-paraspace/blob/c6820a279c64a299a783955749fdc977de8f0449/paraspace-core/contracts/misc/NFTFloorOracle.sol#L326

File: NFTFloorOracle.sol
326:     function _removeFeeder(address _feeder)
327:         internal
328:         onlyWhenFeederExisted(_feeder)
329:     {
330:         uint8 feederIndex = feederPositionMap[_feeder].index;
331:         if (feederIndex >= 0 && feeders[feederIndex] == _feeder) {
332:             feeders[feederIndex] = feeders[feeders.length - 1];
333:             feeders.pop();
334:         }
335:         delete feederPositionMap[_feeder];
336:         revokeRole(UPDATER_ROLE, _feeder);
337:         emit FeederRemoved(_feeder);
338:     }

Thus, anyone can call NFTFloorOracle.removeFeeder function.

Impact

Anyone can remove all feeders from the Oracle. Thus, no one will be able to feed the latest prices to NFTFloorOracle. The entire protocol relies on NFTFloorOracle to function properly. Without the latest price, the market value of NFT cannot be determined, and the collateral will end up being overvalued or undervalued. This causes assets to be liquidated prematurely OR allow users to overborrow.

Additionally, if there is a malicious feeder, the malicious feeder can remove the other feeders and gain full control of the price within ParaSpace.

Recommended Mitigation Steps

Ensure that only owner can remove the feeder from the NFTFloorOracle.

/// @notice Allows owner to remove feeder.
/// @param _feeder feeder to remove
function removeFeeder(address _feeder)
	external
-	onlyWhenFeederExisted(_feeder)
+	onlyRole(DEFAULT_ADMIN_ROLE)
{
	_removeFeeder(_feeder);
}

Lines of code

https://github.com/code-423n4/2022-11-paraspace/blob/c6820a279c64a299a783955749fdc977de8f0449/paraspace-core/contracts/protocol/tokenization/libraries/ApeStakingLogic.sol#L38

Vulnerability details

Background

This section provides a context of the pre-requisite concepts that a reader needs to fully understand the issue.

Split Pair Edge Case In Paired Pool

Assume that Jimmy is the owner of BAYC #8888 and BAKC #9999 NFTs initially. He participated in the Paired Pool and staked + accrued a total of 100 ApeCoin (APE) at this point, as shown in the diagram below.

Jimmy then sold his BAKC #9999 NFT to Ben. When this happens, both parties (Jimmy and Ben) could close out their staking position. Since Ben owns BAKC #9999 now, he can close out Jimmy's position anytime and claim all the accrued APE rewards (2 APE below). While Jimmy will obtain the 98 APE that he staked initially.

The following image is taken from https://youtu.be/_LO-1f9nyjs?t=640

The ApeCoinStaking._withdrawPairNft taken from the official $APE Staking Contract shows that the implementation allows both the BAYC/MAYC owners and BAKC owners to close out the staking position. Refer to Line 976 below.

When the staking position is closed by the BAKC owners, the entire staking amount must be withdrawn. A partial amount is not allowed per Line 981 below. In Line 984, all the accrued APE rewards will be sent to the BAKC owners. In Line 989, all the staked APEs will be withdrawn (unstake) and sent directly to the wallet of the BAYC owners.

https://github.com/code-423n4/2022-11-paraspace/blob/c6820a279c64a299a783955749fdc977de8f0449/paraspace-core/contracts/dependencies/yoga-labs/ApeCoinStaking.sol#L966

File: ApeCoinStaking.sol
966:     function _withdrawPairNft(uint256 mainTypePoolId, PairNftWithAmount[] calldata _nfts) private {
967:         for(uint256 i; i < _nfts.length; ++i) {
968:             uint256 mainTokenId = _nfts[i].mainTokenId;
969:             uint256 bakcTokenId = _nfts[i].bakcTokenId;
970:             uint256 amount = _nfts[i].amount;
971:             address mainTokenOwner = nftContracts[mainTypePoolId].ownerOf(mainTokenId);
972:             address bakcOwner = nftContracts[BAKC_POOL_ID].ownerOf(bakcTokenId);
973:             PairingStatus memory mainToSecond = mainToBakc[mainTypePoolId][mainTokenId];
974:             PairingStatus memory secondToMain = bakcToMain[bakcTokenId][mainTypePoolId];
975: 
976:             require(mainTokenOwner == msg.sender || bakcOwner == msg.sender, "At least one token in pair must be owned by caller");
977:             require(mainToSecond.tokenId == bakcTokenId && mainToSecond.isPaired
978:                 && secondToMain.tokenId == mainTokenId && secondToMain.isPaired, "The provided Token IDs are not paired");
979: 
980:             Position storage position = nftPosition[BAKC_POOL_ID][bakcTokenId];
981:             require(mainTokenOwner == bakcOwner || amount == position.stakedAmount, "Split pair can't partially withdraw");
982: 
983:             if (amount == position.stakedAmount) {
984:                 uint256 rewardsToBeClaimed = _claim(BAKC_POOL_ID, position, bakcOwner);
985:                 mainToBakc[mainTypePoolId][mainTokenId] = PairingStatus(0, false);
986:                 bakcToMain[bakcTokenId][mainTypePoolId] = PairingStatus(0, false);
987:                 emit ClaimRewardsPairNft(msg.sender, rewardsToBeClaimed, mainTypePoolId, mainTokenId, bakcTokenId);
988:             }
989:             _withdraw(BAKC_POOL_ID, position, amount, mainTokenOwner);
990:             emit WithdrawPairNft(msg.sender, amount, mainTypePoolId, mainTokenId, bakcTokenId);
991:         }
992:     }

The following shows that the ApeCoinStaking._withdraw used in the above function will send the unstaked APEs directly to the wallet of BAYC owners. Refer to Line 946 below.

https://github.com/code-423n4/2022-11-paraspace/blob/c6820a279c64a299a783955749fdc977de8f0449/paraspace-core/contracts/dependencies/yoga-labs/ApeCoinStaking.sol#L937

File: ApeCoinStaking.sol
937:     function _withdraw(uint256 _poolId, Position storage _position, uint256 _amount, address _recipient) private {
938:         require(_amount <= _position.stakedAmount, "Can't withdraw more than staked amount");
939: 
940:         Pool storage pool = pools[_poolId];
941: 
942:         _position.stakedAmount -= _amount;
943:         pool.stakedAmount -= _amount;
944:         _position.rewardsDebt -= (_amount * pool.accumulatedRewardsPerShare).toInt256();
945: 
946:         apeCoin.safeTransfer(_recipient, _amount);
947:     }

Who is the owner of BAYC/MAYC in ParaSpace?

The BAYC is held by the NTokenBAYC reserve pool, while the MAYC is held by NTokenMAYC reserve pool. This causes an issue because, as mentioned in the previous section, all the unstaked APE will be sent directly to the wallet of the BAYC/MAYC owners. This will be used as part of the attack path later on.

Does BAKC need to be staked or stored within ParaSpace?

No. For the purpose of APE staking via ParaSpace , BAKC NFT need not be held in the ParaSpace contract for staking, but Bored Apes and Mutant Apes must be collateralized in the ParaSpace protocol. Refer to here. As such, users are free to sell off their BAKC anytime to anyone since it is not being locked up.

Proof of Concept

Using back the same example in the previous section. Assume the following:

• Jimmy is the victim, and Ben is the attacker.
• Jimmy is the owner of BAYC #8888 and BAKC #9999 NFTs initially. He participated in the Paired Pool and staked + accrued a total of 100 ApeCoin (APE).
• Jimmy sold his BAKC #9999 NFT to Ben. There are many ways Ben can obtain the BAKC #9999 NFT. Ben could obtain it via the public marketplace (e.g. Opensea) if Jimmy listed it OR Ben could offer an attractive price to Jimmy to purchase it privately.
• Ben also participates in the APE staking in ParaSpace via his BAYC #0002 and BAKC #0002 NFTs.

Ben will close out Jimmy's position by calling the ApeCoinStaking.withdrawBAKC function of the official $APE Staking Contract to withdraw all the staked APE + accrued APE rewards. As a result, the 98 APE that Jimmy staked initially will be sent directly to the owner of the BAYC #8888 owner. In this case, the BAYC #8888 owner is ParaSpace's NTokenBAYC reserve pool.

At this point, it is important to note that Jimmy's 98 APE is stuck in ParaSpace's NTokenBAYC reserve pool. If anyone can siphon out Jimmy's 98 APE that is stuck in the contract, that person will be able to get free APE.

There exist a way to siphon out all the APE coin that resides in ParaSpace's NTokenBAYC reserve pool. Anyone who participates in APE staking via ParaSpace with a BAYC, which also means that any user staked in the Paired Pool, can trigger the ApeStakingLogic.withdrawBAKC function below by calling the PoolApeStaking.withdrawBAKC function.

Notice that in Line 53 below, it will compute the total balance of APE held by ParaSpace's NTokenBAYC reserve pool contract. In Line 55, it will send all the APE in the pool contract to the recipient. This effectively performs a sweeping of APE stored in the pool contract.

https://github.com/code-423n4/2022-11-paraspace/blob/c6820a279c64a299a783955749fdc977de8f0449/paraspace-core/contracts/protocol/tokenization/libraries/ApeStakingLogic.sol#L38

File: ApeStakingLogic.sol
38:     function withdrawBAKC(
39:         ApeCoinStaking _apeCoinStaking,
40:         uint256 poolId,
41:         ApeCoinStaking.PairNftWithAmount[] memory _nftPairs,
42:         address _apeRecipient
43:     ) external {
44:         ApeCoinStaking.PairNftWithAmount[]
45:             memory _otherPairs = new ApeCoinStaking.PairNftWithAmount[](0);
46: 
47:         if (poolId == BAYC_POOL_ID) {
48:             _apeCoinStaking.withdrawBAKC(_nftPairs, _otherPairs);
49:         } else {
50:             _apeCoinStaking.withdrawBAKC(_otherPairs, _nftPairs);
51:         }
52: 
53:         uint256 balance = _apeCoinStaking.apeCoin().balanceOf(address(this));
54: 
55:         _apeCoinStaking.apeCoin().safeTransfer(_apeRecipient, balance);
56:     }

Thus, after Ben closes out Jimmy's position by calling the ApeCoinStaking.withdrawBAKC function and causes the 98 APE to be sent to ParaSpace's NTokenBAYC reserve pool contract, Ben immediately calls the PoolApeStaking.withdrawBAKC function against his own BAYC #0002 and BAKC #0002 NFTs. This will result in all of Jimmy's 98 APE being swept to his wallet. Bob effectively steals Jimmy's 98 APE.

Impact

ApeCoin of ParaSpace users can be stolen.

Recommended Mitigation Steps

Consider the potential side effects of the split pair edge case in the pair pool when implementing the APE staking feature in ParaSpace.

The official APE staking contract has been implemented recently, and only a few protocols have integrated with it. Thus, the edge cases are not widely understood and are prone to errors.

To mitigate this issue, instead of returning BAKC NFT to the users after staking, consider locking up BAKC NFT in the ParaSpace contract as part of the user's collateral. In this case, the user will not be able to sell off their BAKC, and the "Split Pair Edge Case In Paired Pool" scenario will not happen.

Lines of code

https://github.com/code-423n4/2022-11-paraspace/blob/c6820a279c64a299a783955749fdc977de8f0449/paraspace-core/contracts/protocol/tokenization/NTokenMoonBirds.sol#L63

Vulnerability details

Proof-of-Concept

The NTokenMoonBirds Reserve Pool only allows Moonbird NFT to be sent to the pool contract as shown in Line 70 below.

https://github.com/code-423n4/2022-11-paraspace/blob/c6820a279c64a299a783955749fdc977de8f0449/paraspace-core/contracts/protocol/tokenization/NTokenMoonBirds.sol#L63

File: NTokenMoonBirds.sol
63:     function onERC721Received(
64:         address operator,
65:         address from,
66:         uint256 id,
67:         bytes memory
68:     ) external virtual override returns (bytes4) {
69:         // only accept MoonBird tokens
70:         require(msg.sender == _underlyingAsset, Errors.OPERATION_NOT_SUPPORTED);
71: 
72:         // if the operator is the pool, this means that the pool is transferring the token to this contract
73:         // which can happen during a normal supplyERC721 pool tx
74:         if (operator == address(POOL)) {
75:             return this.onERC721Received.selector;
76:         }
77: 
78:         // supply the received token to the pool and set it as collateral
79:         DataTypes.ERC721SupplyParams[]
80:             memory tokenData = new DataTypes.ERC721SupplyParams[](1);
81: 
82:         tokenData[0] = DataTypes.ERC721SupplyParams({
83:             tokenId: id,
84:             useAsCollateral: true
85:         });
86: 
87:         POOL.supplyERC721FromNToken(_underlyingAsset, tokenData, from);
88: 
89:         return this.onERC721Received.selector;
90:     }

Note that the NTokenMoonBirds Reserve Pool is the holder/owner of all Moonbird NFTs within ParaSpace. If there is any airdrop for Moonbird NFT, the airdrop will be sent to the NTokenMoonBirds Reserve Pool.

However, due to the validation in Line 70, the NTokenMoonBirds Reserve Pool will not be able to receive any airdrop (e.g. Moonbirds Oddities NFT) other than the Moonbird NFT. In summary, if any NFT other than Moonbird NFT is sent to the pool, it will revert.

For instance, Moonbirds Oddities NFT has been airdropped to Moonbird NFT nested stakers in the past. With the nesting staking feature, more airdrops are expected in the future. In this case, any users who collateralized their nested Moonbird NFT within ParaSpacwill lose their opportunities to claim the airdrop.

Impact

Loss of assets for the user. Users will not be able to receive any airdropped assets for their nested Moonbird NFT.

Recommended Mitigation Steps

Update the contract to allow airdrops to be received by the NTokenMoonBirds Reserve Pool.

function onERC721Received(
	address operator,
	address from,
	uint256 id,
	bytes memory
) external virtual override returns (bytes4) {
-	// only accept MoonBird tokens
-	require(msg.sender == _underlyingAsset, Errors.OPERATION_NOT_SUPPORTED);

	// if the operator is the pool, this means that the pool is transferring the token to this contract
	// which can happen during a normal supplyERC721 pool tx
	if (operator == address(POOL)) {
		return this.onERC721Received.selector;
	}

+	if msg.sender == _underlyingAsset(
        // supply the received token to the pool and set it as collateral
        DataTypes.ERC721SupplyParams[]
            memory tokenData = new DataTypes.ERC721SupplyParams[](1);

        tokenData[0] = DataTypes.ERC721SupplyParams({
            tokenId: id,
            useAsCollateral: true
        });
+	)

	POOL.supplyERC721FromNToken(_underlyingAsset, tokenData, from);

	return this.onERC721Received.selector;
}

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