code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Description

executeAirdrop() is a function admin may call in order to collect airdrops for NFTs held in ParaSpace's nToken contract.

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);
}

The way the function is implemented, admin can call any function with any parameters, making it an incredible rugging vector risk. Admin has no issue calling ERC721 transfer operation on any of it's held assets and completely stealing the underlying value of the NToken.

Impact

Compromised admin can instantly take all NFTs held in NToken contracts

Proof of Concept

Tools Used

Manual audit

Recommended Mitigation Steps

executeAirdrop should be refactored to be time-locked. The first call will lock the arguments into place, then after X time has passed the second call will execute it. In this way, users may withdraw their NFTs from the contract before it is too late.

c4-judge commented 1 year ago

dmvt marked the issue as duplicate of #54