Users can withdraw more funds if the party has tokens with multiple addresses.

[code423n4]

Lines of code

https://github.com/code-423n4/2023-05-party/blob/f6f80dde81d86e397ba4f3dedb561e23d58ec884/contracts/party/PartyGovernanceNFT.sol#L293

Vulnerability details

Impact

Users can withdraw more funds if the party has tokens with multiple addresses.

Proof of Concept

Users can burn their party NFTs and take the share of the party's funds.

    function rageQuit(
        uint256[] calldata tokenIds,
        IERC20[] calldata withdrawTokens,
        address receiver
    ) external {
        ...
            // Withdraw fair share of tokens from the party.
            IERC20 prevToken;
            for (uint256 j; j < withdrawTokens.length; ++j) { //@audit mutiple addresses token?
                IERC20 token = withdrawTokens[j];

                // Prevent null and duplicate transfers.
                if (prevToken >= token) revert InvalidTokenOrderError();

                prevToken = token;

                // Check if token is ETH.
                if (address(token) == ETH_ADDRESS) {
                    // Transfer fair share of ETH to receiver.
                    uint256 amount = (address(this).balance * shareOfVotingPower) / 1e18;
                    if (amount != 0) {
                        payable(receiver).transferEth(amount);
                    }
                } else {
                    // Transfer fair share of tokens to receiver.
                    uint256 amount = (token.balanceOf(address(this)) * shareOfVotingPower) / 1e18;
                    if (amount != 0) {
                        token.compatTransfer(receiver, amount);
                    }
                }
            }
        }
    }

Users can input custom token addresses and receive them according to their shares.

Btw there are some tokens with multiple addresses and the party might contain such tokens while executing several proposals like ArbitraryCallsProposal.

In this case, a malicious user might input both addresses for one token and charge more funds than he should.

Tools Used

Manual Review

Recommended Mitigation Steps

I think we should add more validations in rageQuit() like pre/post balance checking.

Assessed type

ERC20

[0xble]

Duplicate of #14

[c4-judge]

thereksfour marked the issue as duplicate of #14

[c4-judge]

thereksfour marked the issue as satisfactory

[gjaldon]

Are there actual proxied tokens that implement proxies call like this?

 let result := call(gas(), _impl, callvalue(), ptr, add(calldatasize(), 32), 0, 0)

Don't proxies normally use delegatecall? Looked at https://github.com/d-xo/weird-erc20#multiple-token-addresses but it doesn't have an example of a token that uses it.