[Lines of code](https://github.com/code-423n4/2023-10-party/blob/b23c65d62a20921c709582b0b76b387f2bb9ebb5/contracts/party/PartyGovernanceNFT.sol#L255-L258) # Vulnerability details ## Impact If totalVotingPower < mintedVotingPower, the contract would loses tokens deposited in the contract, the Authority can stop rageQuit (Authority doesn’t have permission to do this), and the protocol stops functioning properly. ## Proof of Concept The decreaseTotalVotingPower function does not check if the decreased totalVotingPower is less than the mintedVotingPower. This can cause a number of problems. function decreaseTotalVotingPower(uint96 votingPower) external { _assertAuthority(); @> _getSharedProposalStorage().governanceValues.totalVotingPower -= votingPower; } 1. Taking more tokens when running rageQuit 1. User can take more tokens than the percentage of votingPower user actually have. 2. In the worst case, user can remove all the tokens deposited into the contract. (In an extreme example, if Authority call decreaseTotalVotingPower to go from totalVotingPower : 100, mintedVotingPower: 50 to totalVotingPower : 25, mintedVotingPower: 50, user can take the entire amount with only 25 voting power.) function rageQuit( uint256[] calldata tokenIds, IERC20[] calldata withdrawTokens, uint256[] calldata minWithdrawAmounts, address receiver ) external { ... uint256[] memory withdrawAmounts = new uint256[](withdrawTokens.length); { 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; } } } ... } function getVotingPowerShareOf(uint256 tokenId) public view returns (uint256) { uint256 totalVotingPower = _getSharedProposalStorage().governanceValues.totalVotingPower; return @> totalVotingPower == 0 ? 0 : (votingPowerByTokenId[tokenId] * 1e18) / totalVotingPower; } 2. Authority can stop rageQuit 1. Only Host can stop the rageQuit (setRageQuit) 2. Users with votingPower > totalVotingPower will not be able to rageQuit because of underflow. Authority can stop rageQuit by setting very small totalVotingPower function setRageQuit(uint40 newRageQuitTimestamp) external { @> _assertHost(); ... } function rageQuit( uint256[] calldata tokenIds, IERC20[] calldata withdrawTokens, uint256[] calldata minWithdrawAmounts, address receiver ) external { ... { // 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; } ... } 3. When voting, votes can be counted as more valuable than they actually are, causing the vote to pass or be unanimous. 1. _areVotesPassing 1. Small number of voting power can make proposal success function _areVotesPassing( uint96 voteCount, uint96 totalVotingPower, uint16 passThresholdBps ) private pure returns (bool) { @> return (uint256(voteCount) * 1e4) / uint256(totalVotingPower) >= uint256(passThresholdBps); } 2. _isUnanimousVotes 1. The proposal will be executed immediately due to unanimity, even with a small number of voting power. function _isUnanimousVotes( uint96 totalVotes, uint96 totalVotingPower ) private pure returns (bool) { @> uint256 acceptanceRatio = (totalVotes * 1e4) / totalVotingPower; // If >= 99.99% acceptance, consider it unanimous. // The minting formula for voting power is a bit lossy, so we check // for slightly less than 100%. return acceptanceRatio >= 0.9999e4; } This is PoC for one of the issues above, Authority can stop rageQuit function testDecreaseTotalVotingPower_PoC() external { (Party party, , ) = partyAdmin.createParty( partyImpl, PartyAdmin.PartyCreationMinimalOptions({ host1: address(this), host2: address(0), passThresholdBps: 5100, totalVotingPower: 100, preciousTokenAddress: address(toadz), preciousTokenId: 1, rageQuitTimestamp: ENABLE_RAGEQUIT_PERMANENTLY, feeBps: 0, feeRecipient: payable(0) }) ); address authority = address(partyAdmin); // Mint address member = _randomAddress(); vm.prank(authority); uint256 tokenId = party.mint(member, 50, member); vm.deal(address(party), 1 ether); IERC20[] memory tokens = new IERC20[](4); tokens[0] = IERC20(address(new DummyERC20())); tokens[1] = IERC20(address(new DummyERC20())); tokens[2] = IERC20(address(new DummyERC20())); tokens[3] = IERC20(ETH_ADDRESS); // Sort the addresses from lowest to highest. for (uint256 i; i < tokens.length; ++i) { for (uint256 j = 0; j < tokens.length - i - 1; j++) { if (address(tokens[j]) > address(tokens[j + 1])) { IERC20 temp = tokens[j]; tokens[j] = tokens[j + 1]; tokens[j + 1] = temp; } } } uint256[] memory minWithdrawAmounts = new uint256[](4); uint96[] memory balances = new uint96[](3); for (uint256 i; i < balances.length; ++i) { balances[i] = uint96(_randomRange(10, type(uint96).max)); DummyERC20(address(tokens[i])).deal(address(party), balances[i]); } uint256[] memory tokenIds = new uint256[](1); tokenIds[0] = tokenId; // decrease total voting power uint96 votingPower = 80; vm.prank(authority); party.decreaseTotalVotingPower(votingPower); assertEq(party.getGovernanceValues().totalVotingPower, 20); // try to rageQuit but fail vm.expectRevert(stdError.arithmeticError); vm.prank(member); party.rageQuit(tokenIds, tokens, minWithdrawAmounts, member); } ## Tools Used Manual Review ## Recommended Mitigation Steps function decreaseTotalVotingPower(uint96 votingPower) external { _assertAuthority(); + require(totalVotingPower - votingPower >= mintedVotingPower); _getSharedProposalStorage().governanceValues.totalVotingPower -= votingPower; } ## Assessed type Other --- The text was updated successfully, but these errors were encountered: All reactions