code-423n4 / 2023-05-ajna-findings

2 stars 0 forks source link

Race condition issue on equally preferred proposal slates #129

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-05-ajna/blob/276942bc2f97488d07b887c8edceaaab7a5c3964/ajna-grants/src/grants/base/StandardFunding.sol#L317

Vulnerability details

Impact

To decide if a given slate of proposals is to become the new fundedProposalsSlates, the code check the following:

// check if slate of proposals is better than the existing slate, and is thus the new top slate
newTopSlate_ = currentSlateHash == 0 || (currentSlateHash!= 0 && sum > _sumProposalFundingVotes(_fundedProposalSlates[currentSlateHash]));v

sum is the summation of the votes received on each proposal of the slate. The formula says that in order to update a slate, or is the first one, or its sum is greater than the previous. If it happens to exist more than one slate which maximizes the sum of the net number of votes, that is, all of them have the same sum and this sum is the highest that can exists given the current proposals. Then, the first to get submitted is going to win.

Proof of Concept

Proposal 1: -5 votes Proposal 2: 10 votes Proposal 3: 2 votes Proposal 4: 2 votes Proposal 5: 8 votes Proposal 6: -3 votes Proposal 7: 4 votes Proposal 8: 9 votes Proposal 9: 7 votes Proposal 10: 3 votes

Slate 1: [Proposal 2, Proposal 3, Proposal 8], summation of votes = 21 Slate 2: [Proposal 2, Proposal 4, Proposal 8], summation of votes = 21

Assume that the 2 slates above are the ones with the max possible summation of votes which satisfies the budget constraint. Whichever of both that gets submitted first, will win.

Tools Used

Manual Review

Recommended Mitigation Steps

I recommend using an extra criteria if the sum of two or more slates of proposals are equal. For example, which requested less tokens.

Assessed type

Other

c4-judge commented 1 year ago

Picodes marked the issue as duplicate of #286

c4-judge commented 1 year ago

Picodes marked the issue as selected for report

c4-sponsor commented 1 year ago

MikeHathaway marked the issue as sponsor disputed

MikeHathaway commented 1 year ago

This was previously acknowledged behavior and alternative tie breaking schemes are also arbitrary, as pointed out in duplicate issues.

c4-judge commented 1 year ago

Picodes changed the severity to QA (Quality Assurance)

c4-judge commented 1 year ago

Picodes marked the issue as not selected for report