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

2 stars 0 forks source link

Treasury Depletion Halting Execution of Extraordinary Proposal in Simultaneous PFM and EFM Scenario. #399

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-grants/src/grants/base/ExtraordinaryFunding.sol#L164-L178 https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-grants/src/grants/base/ExtraordinaryFunding.sol#L222-L227

Vulnerability details

Impact

The issue arises when the distribution of rewards for the primary funding mechanism occurs after the voting process of the extraordinary proposal. This sequence of events can result in changes to the treasury balance, which in turn can cause the checks within the extraordinary proposal's execution process to fail, leading to the extraordinary proposal not being able to be executed at all. This also leads to loss of resources that are used in the massive voting process.

As confirmed with the developers that it is completely possible that Primary Funding Mechanism and Extraordinary Funding Mechanism can happen in parallel, therefore this scenario is quite possible.

Proof of Concept

Gist to full code

Note: Save this file to "2023-05-ajna/ajna-grants/test/unit". Run the PoC with " forge test --match-contract DoS -vvv"

The function executeExtraordinary() in ExtraordinaryFunding.sol has a check:

.
.
.

if (proposal.executed || !_extraordinaryProposalSucceeded(proposalId_, tokensRequested)) revert ExecuteExtraordinaryProposalInvalid();

.
.
.

This leads us to _extraordinaryProposalSucceeded() where we have two conditions that must be met in order for the proposal to be executed out of which only one is of the interest:

.
.
.

(votesReceived >= tokensRequested_ + _getSliceOfNonTreasury(minThresholdPercentage))

.
.
.

Because it leads us to the core of this issue at _getSliceOfNonTreasury():

    function _getSliceOfNonTreasury(
        uint256 percentage_
    ) internal view returns (uint256) {
        uint256 totalAjnaSupply = IERC20(ajnaTokenAddress).totalSupply();
        return Maths.wmul(totalAjnaSupply - treasury, percentage_); //@audit here
    }

This function is using the current value of treasury variable which will change when the funds are distributed.

The number of votes required is inversely propotional to the treasury balance. So, if the treasury balance decreases due to sending out rewards, then the number of votes required will increase. But, now that the voting has ended, it is not possible to vote and the execution of that proposal will not happen anymore.

In the provided Proof of Concept (PoC) code, the function testFailProposeAndExecuteExtraordinary() demonstrates this issue.

During the voting process of the extraordinary proposal, the PFM is running, and its challenge period has already ended. Meanwhile, the EFM achieves a voting quorum of just over 50% and the voting period for the EFM ends.

However, when the majority of delegates claim their share of the rewards from the PFM, it causes a depletion of approximately 7-8% of the treasury balance. As a result, the return value of _getSliceOfNonTreasury() increases due to the reduced treasury balance.

This change in the return value invalidates the condition necessary for the execution of the extraordinary proposal, rendering it permanently prevented from being executed. Since the voting process has already concluded, there is no possibility of revisiting the vote and addressing the issue.

Tools Used

Manual review + Foundry

Recommended Mitigation Steps

Assessed type

DoS

c4-judge commented 1 year ago

Picodes marked the issue as duplicate of #489

c4-judge commented 1 year ago

Picodes marked the issue as unsatisfactory: Invalid