PoolAdmin can steal NFT from NTokens

[code423n4]

Lines of code

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

Vulnerability details

Impact

The executeAirdrop function allow pool admin to execute arbitrary call to arbitrary contract, including a transferFrom call to the underlying NFT contract. This can be used by the pool admin to steal NFT inside the NToken contracts.

Since the rescueERC721 have a check to make sure the target is not _underlyingAsset, it seems like executeAirdrop should have the same check.

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

        require(
            token != _underlyingAsset,
            Errors.UNDERLYING_ASSET_CAN_NOT_BE_TRANSFERRED
        );

Proof of Concept

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

    function executeAirdrop(
        address airdropContract,
        bytes calldata airdropParams
    ) external override onlyPoolAdmin {
        require(
            airdropContract != address(0),
            Errors.INVALID_AIRDROP_CONTRACT_ADDRESS
        );
        require(airdropParams.length >= 4, Errors.INVALID_AIRDROP_PARAMETERS);

        // call project airdrop contract
        Address.functionCall(
            airdropContract,
            airdropParams,
            Errors.CALL_AIRDROP_METHOD_FAILED
        );

        emit ExecuteAirdrop(airdropContract);
    }

Recommended Mitigation Steps

        require(
            airdropContract != _underlyingAsset,
            Errors.UNDERLYING_ASSET_CAN_NOT_BE_TRANSFERRED
        );

[c4-judge]

dmvt marked the issue as duplicate of #54

[c4-judge]

dmvt marked the issue as satisfactory