code-423n4 / 2022-09-party-findings

2 stars 0 forks source link

You can give address(0) _mint(), confused with _burn() #238

Closed code423n4 closed 1 year ago

code423n4 commented 2 years ago

Lines of code

https://github.com/PartyDAO/party-contracts-c4/blob/main/contracts/crowdfund/AuctionCrowdfund.sol#L126 https://github.com/PartyDAO/party-contracts-c4/blob/main/contracts/crowdfund/CrowdfundNFT.sol#L141-L148

Vulnerability details

Impact

You can give address(0) _mint(), confused with _burn()

Proof of Concept

For example, the Initialize () of AuctionCrowdfund does not determine whether initialContributor is 0, so the address(0) can be Mint tokenid.

  //contracts/crowdfund/AuctionCrowdfund.sol
  function initialize(AuctionCrowdfundOptions memory opts)
      external
      payable
      onlyConstructor
  {
        ...
      Crowdfund._initialize(CrowdfundOptions({
          ...
          initialContributor: opts.initialContributor,
      }));

  --------------->

    //contracts/crowdfund/Crowdfund.sol
    function _initialize(CrowdfundOptions memory opts)
        internal
    {
              ...
            _contribute(opts.initialContributor, initialBalance, opts.initialDelegate, 0, "");
        }
    }

    //contracts/crowdfund/Crowdfund.sol
    function _contribute(
        address contributor,
        uint96 amount,
        address delegate,
        uint96 previousTotalContributions,
        bytes memory gateData
    )
        internal
    {
            ...
            if (numContributions == 0) {
                _mint(contributor);
            }
        }
    }
    //contracts/crowdfund/CrowdfundNFT.sol
    function _mint(address owner) internal returns (uint256 tokenId)
    {
        tokenId = uint256(uint160(owner));
        if (_owners[tokenId] != owner) {
            _owners[tokenId] = owner;
            emit Transfer(address(0), owner, tokenId);
        }
    }

The corresponding relation here is' _owners[tokenId]=address(0) '

Just like in _burn(), it causes interference

    function _burn(address owner) internal {
        uint256 tokenId = uint256(uint160(owner));
        if (_owners[tokenId] == owner) {
            _owners[tokenId] = address(0);
            emit Transfer(owner, address(0), tokenId);
            return;
        }
        revert AlreadyBurnedError(owner, tokenId);
    }

Tools Used

vscode

Recommended Mitigation Steps

    function _mint(address owner) internal returns (uint256 tokenId)
    {
        require(owner != address(0),"owner is address(0)");
        tokenId = uint256(uint160(owner));
        if (_owners[tokenId] != owner) {
            _owners[tokenId] = owner;
            emit Transfer(address(0), owner, tokenId);
        }
    }
merklejerk commented 1 year ago

Duplicate of #105