Lines of code
<https://github.com/PartyDAO/party-contracts-c4/blob/3896577b8f0fa16cba129dc2867aba786b730c1b/contracts/proposals/ArbitraryCallsProposal.sol#L104&gt;

Vulnerability details

Impact

Detailed description of the impact of this finding.

Let’s look into the implementation in ArbitraryCallsProposal.sol

        // Check that the call is not prohibited.
        if (!_isCallAllowed(call, isUnanimous, preciousTokens, preciousTokenIds)) {
            revert CallProhibitedError(call.target, call.data);
        }
        // Check that we have enough ETH to execute the call.
        if (ethAvailable < call.value) {
            revert NotEnoughEthAttachedError(call.value, ethAvailable);
        }
        // Execute the call.
        (bool s, bytes memory r) = call.target.call{ value: call.value }(call.data);

the _isCallAllowed make sure that malicious actor cannot approve a malicious address spending party’s nft for ERC721 token.

                 if (selector == IERC721.approve.selector) {
                    // Can only call approve() on the precious if the operator is null.
                    (address op, uint256 tokenId) = _decodeApproveCallDataArgs(call.data);
                    if (op != address(0)) {
                        return !LibProposal.isTokenIdPrecious(
                            IERC721(call.target),
                            tokenId,
                            preciousTokens,
                            preciousTokenIds
                        );
                    }
                // Can only call setApprovalForAll() on the precious if
                // toggling off.
                } else if (selector == IERC721.setApprovalForAll.selector) {
                    (, bool isApproved) = _decodeSetApprovalForAllCallDataArgs(call.data);
                    if (isApproved) {
                        return !LibProposal.isTokenPrecious(IERC721(call.target), preciousTokens);
                    }
                }

note the if condition

 if (selector == IERC721.approve.selector)

but the malicious member can create a proposal to approve a malicious contract to spend ERC1155 NFT and ERC20 tokens.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

Case 1

1. a ERC1155 NFT is purchased and party is created.

2. a malicious member create a proposal to approve a malicious smart contract to spend the ERC1155 token in the party contract
   via the ArbitraryCallsProposal.sol

       // Execute the call.
      (bool s, bytes memory r) = call.target.call{ value: call.value }(call.data);
   

the call.target would be ERC1155 nft contract, the payload would be the setApprovalFor function signature + the malicious smart contract call data.

3. the proposal is executed and a malicious smart contract transfer the NFT out.

case 2

1. a ERC721 NFT is purchase and party is created.

2. a FractionalizedProposal is passed and the NFT is fractionalized into N supply of ERC20 token share.

3. A malicious member create a proposal to approve malicious spending for a malicious contract to spend the ERC20 token in the party contract

by calling

    // Execute the call.
   (bool s, bytes memory r) = call.target.call{ value: call.value }(call.data);

the target would be the ERC20 address, the call data and payload is the ERC20 token approve function signature + the malicious smart contract address call data.

4. the proposal is executed and the ERC20 token are transfered out.

Tools Used

Manual Review
Foundry

Recommended Mitigation Steps

We recommend the project check if the party still holds the NFT in the party contract after each call

We recommend the project disable the ERC20 token approval.

---

The text was updated successfully, but these errors were encountered:

All reactions