Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Magnificent Tortilla Eel - Missing Maximum Participants Check in buyVotes() and Missing Remove from participants[] in sellVotes(). #146

Open
sherlock-admin2 opened this issue Dec 30, 2024 · 0 comments

Comments

@sherlock-admin2
Copy link

Magnificent Tortilla Eel

High

Missing Maximum Participants Check in buyVotes() and Missing Remove from participants[] in sellVotes().

Summary

The buyVotes() function does not check for maximium value of participants.length.
The sellVotes() function does not remove users who have sold all their votes.

Root Cause

https://github.com/sherlock-audit/2024-12-ethos-update/blob/main/ethos/packages/contracts/contracts/ReputationMarket.sol#L440
https://github.com/sherlock-audit/2024-12-ethos-update/blob/main/ethos/packages/contracts/contracts/ReputationMarket.sol#L539

Internal pre-conditions

N/A

External pre-conditions

N/A

Attack Path

N/A

Impact

https://github.com/sherlock-audit/2024-12-ethos-update/blob/main/ethos/packages/contracts/contracts/ReputationMarket.sol#L26-L30

 * Graduation: the intent is that upon graduation, each holder of trust and distrust votes receives equivalent ERC-20 tokens
 * representing their position. These tokens will be freely tradable, without the reciprocal pricing mechanism of this contract.
 * A price floor will be established by Ethos, offering to buy back the new ERC-20 tokens at their final vote price upon graduation,
 * ensuring participants don't incur losses due to the transition. Only Ethos, through a designated contract, will be authorized to
 * graduate markets and withdraw funds to initiate this conversion process. This conversion contract is not yet implemented.

The participants.length will increase continusly.
If the participants.length increase excessively, after the graduation, each holder of TRUST and DISTURST votes may not receives equivalent ERC-20 tokens due to running out of gas.

PoC

ReputationMarket.sol
440:    function buyVotes(
            uint256 profileId,
            bool isPositive,
            uint256 maxVotesToBuy,
            uint256 minVotesToBuy
        ) public payable whenNotPaused activeMarket(profileId) nonReentrant {
            _checkMarketExists(profileId);
            // preliminary check to ensure this is enough money to buy the minimum requested votes.
            (, , , uint256 total) = _calculateBuy(markets[profileId], isPositive, minVotesToBuy);
            if (total > msg.value) revert InsufficientFunds();

            (
                uint256 purchaseCostBeforeFees,
                uint256 protocolFee,
                uint256 donation,
                uint256 totalCostIncludingFees
            ) = _calculateBuy(markets[profileId], isPositive, maxVotesToBuy);
            uint256 currentVotesToBuy = maxVotesToBuy;
            // if the cost is greater than the maximum votes to buy,
            // decrement vote count and recalculate until we identify the max number of votes they can afford
            while (totalCostIncludingFees > msg.value) {
                currentVotesToBuy--;
                (purchaseCostBeforeFees, protocolFee, donation, totalCostIncludingFees) = _calculateBuy(
                    markets[profileId],
                    isPositive,
                    currentVotesToBuy
                );
            }

            // Update market state
            markets[profileId].votes[isPositive ? TRUST : DISTRUST] += currentVotesToBuy;
            votesOwned[msg.sender][profileId].votes[isPositive ? TRUST : DISTRUST] += currentVotesToBuy;

            // Add buyer to participants if not already a participant
            if (!isParticipant[profileId][msg.sender]) {
                participants[profileId].push(msg.sender);
                isParticipant[profileId][msg.sender] = true;
            }

            // tally market funds
            marketFunds[profileId] += purchaseCostBeforeFees;

            // Distribute the fees
            applyFees(protocolFee, donation, profileId);

            // Calculate and refund remaining funds
            uint256 refund = msg.value - totalCostIncludingFees;
            if (refund > 0) _sendEth(refund);
            emit VotesBought(
                profileId,
                msg.sender,
                isPositive,
                currentVotesToBuy,
                totalCostIncludingFees,
                block.timestamp
            );
            _emitMarketUpdate(profileId);
        }
539:    function sellVotes(
            uint256 profileId,
            bool isPositive,
            uint256 votesToSell,
            uint256 minimumVotePrice
        ) public whenNotPaused activeMarket(profileId) nonReentrant {
            _checkMarketExists(profileId);
            (uint256 proceedsBeforeFees, uint256 protocolFee, uint256 proceedsAfterFees) = _calculateSell(
                markets[profileId],
                profileId,
                isPositive,
                votesToSell
            );

            uint256 pricePerVote = votesToSell > 0 ? proceedsBeforeFees / votesToSell : 0;
            if (pricePerVote < minimumVotePrice) {
                revert SellSlippageLimitExceeded(minimumVotePrice, pricePerVote);
            }

            markets[profileId].votes[isPositive ? TRUST : DISTRUST] -= votesToSell;
            votesOwned[msg.sender][profileId].votes[isPositive ? TRUST : DISTRUST] -= votesToSell;
            // tally market funds
            marketFunds[profileId] -= proceedsBeforeFees;

            // apply protocol fees
            applyFees(protocolFee, 0, profileId);

            // send the proceeds to the seller
            _sendEth(proceedsAfterFees);

            emit VotesSold(
                profileId,
                msg.sender,
                isPositive,
                votesToSell,
                proceedsAfterFees,
                block.timestamp
            );
            _emitMarketUpdate(profileId);
        }

Mitigation

Consider adding a check for maximium value of paricipants.length and removing users who have sold all their votes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant