code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/NFTCollection.sol#L197-L199

Vulnerability details

Impact

When minting an NFT, the minter can use special functions within the NFTCollection.sol contract to specify the royalty recipient address for this NFT. One of this special functions is NFTCollection.mintWithCreatorPaymentFactory. This function expects as an argument an address paymentAddressFactory which is expected to be a factory contract returning the address to use for royalty payments. This contract is called with arbitrary call data by using AddressLibrary.callAndReturnContractAddress.

AddressLibrary.callAndReturnContractAddress calls an external contract with arbitrary data and parse the return value into an address. Any external call which does not return an address will revert. Nevertheless, as the function call data and the contract address are completely arbitrary and user (collection creator/owner) controlled, this could impose security issues that the protocol owner is currently unaware of.

Proof of Concept

NFTCollection.sol#L197-L199

function mintWithCreatorPaymentFactory(
  string calldata tokenCID,
  address paymentAddressFactory,
  bytes calldata paymentAddressCallData
) public returns (uint256 tokenId) {
  address payable tokenCreatorPaymentAddress = paymentAddressFactory.callAndReturnContractAddress(
    paymentAddressCallData
  );
  tokenId = mintWithCreatorPaymentAddress(tokenCID, tokenCreatorPaymentAddress);
}

Tools Used

Manual review

Recommended mitigation steps

Consider adding a protocol-wide allowlist for approved contract addresses which can be used as payment address factories.

HardlyDifficult commented 2 years ago

Dupe of https://github.com/code-423n4/2022-08-foundation-findings/issues/248

HickupHH3 commented 2 years ago

part of warden's QA: #161

code-423n4 / 2022-08-foundation-findings