Digital garden

code4rena-2023-11-party-dao-m05

4 min read

code4rena-2023-11-party-dao-m05

[M-05] PartyGovernanceNFT.rageQuit() can lead to token loss for users when dealing with zero-balance ERC20 during a rageQuit()

보고서

Summary

amount가 0일 때는 유저가 요청한 최소 금액을 충당하는 지 확인하지 않기 때문에 받을 수 있는 amount가 0일 때에는 트랜잭션이 취소되지 않는다. 유저의 NFT만 잃고 토큰은 받지 못하게 된다.

Keyword

off by one error

Vulnerability

rageQuit에서 유저가 원하는 최소 액수를 설정한다. 유저가 요청한 최소 액수보다 적은 양을 받게 된다면 이 트랜잭션은 취소되어야 한다. 하지만 amount > 0 일 때만 amount < minAmount 인지 확인한다. 즉, amount 가 0일 때는 유저가 요청한 최소값을 충당하는 지 확인하지 않는다.

withdrawTokens로 요청한 토큰이 고갈되어 얻을 수 있는 amount가 0이 되는 상황이 발생할 수 있다. 이 때 유저는 자신의 NFT만 소각당하고 그에 상응하는 대가는 얻을 수 없게 된다.

    function rageQuit(
        uint256[] calldata tokenIds,
@>      IERC20[] calldata withdrawTokens,
@>      uint256[] calldata minWithdrawAmounts,
        address receiver
    ) external {
        {
            IERC20 prevToken;
            for (uint256 i; i < withdrawTokens.length; ++i) {
                // Check if order of tokens to transfer is valid.
                // Prevent null and duplicate transfers.
                if (prevToken >= withdrawTokens[i]) revert InvalidTokenOrderError();
 
                prevToken = withdrawTokens[i];
 
                // Check token's balance.
@>              uint256 balance = address(withdrawTokens[i]) == ETH_ADDRESS
@>                  ? address(this).balance
@>                  : withdrawTokens[i].balanceOf(address(this));
 
                // Add fair share of tokens from the party to total.
                for (uint256 j; j < tokenIds.length; ++j) {
                    // Must be retrieved before burning the token.
@>                  withdrawAmounts[i] += (balance * getVotingPowerShareOf(tokenIds[j])) / 1e18;
                }
            }
        }
        {
            // Burn caller's party cards. This will revert if caller is not the
            // the owner or approved for any of the card they are attempting to
            // burn, not an authority, or if there are duplicate token IDs.
@>          uint96 totalVotingPowerBurned = _burnAndUpdateVotingPower(tokenIds, !isAuthority_);
 
            // Update total voting power of party.
@>          _getSharedProposalStorage().governanceValues.totalVotingPower -= totalVotingPowerBurned;
        }
        {
            uint16 feeBps_ = feeBps;
            for (uint256 i; i < withdrawTokens.length; ++i) {
                IERC20 token = withdrawTokens[i];
                uint256 amount = withdrawAmounts[i];
 
                ...
 
@>              if (amount > 0) {
                    uint256 minAmount = minWithdrawAmounts[i];
 
                    // Check amount is at least minimum.
@>                  if (amount < minAmount) {
                        revert BelowMinWithdrawAmountError(amount, minAmount);
                    }
 
                    // Transfer token from party to recipient.
                    if (address(token) == ETH_ADDRESS) {
                        payable(receiver).transferEth(amount);
                    } else {
                        token.compatTransfer(receiver, amount);
                    }
                }
            }
        }
        ...
    }

Impact

minWithdrawAmounts 배열이 무시되고 보상 없이 사용자의 NFT만 잃게 된다.

Mitigation

--- a/contracts/party/PartyGovernanceNFT.sol
+++ b/contracts/party/PartyGovernanceNFT.sol
@@ -418,17 +418,17 @@ contract PartyGovernanceNFT is PartyGovernance, ERC721, IERC2981 {
                     // Transfer fee to fee recipient.
                     if (address(token) == ETH_ADDRESS) {
                         payable(feeRecipient).transferEth(fee);
                     } else {
                         token.compatTransfer(feeRecipient, fee);
                     }
                 }
 
-                if (amount > 0) {
+                if (amount >= 0) {
                     uint256 minAmount = minWithdrawAmounts[i];
 
                     // Check amount is at least minimum.
                     if (amount < minAmount) {
                         revert BelowMinWithdrawAmountError(amount, minAmount);
                     }
 
                     // Transfer token from party to recipient.

tags: bughunting, party dao, smart contract, solidity, off by one error, severity medium

Graph View