Ajna Protocol - 0x73696d616f's results

Lines of code

https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-grants/src/grants/base/StandardFunding.sol#L286 https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-grants/src/grants/base/StandardFunding.sol#L541 https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-grants/src/grants/base/StandardFunding.sol#L673 https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-grants/src/grants/base/StandardFunding.sol#L891

Vulnerability details

Impact

Reduces decentralization significantly and discourages delegates with less token power to vote.

Proof of Concept

The current math gives delegates rewards based on the square of their votes. Thus, accounts with higher number of votes will be rewarded a bigger number of rewards, leading to less decentralization.

Add the following test to StandardFunding.t.sol

function test_POC_WhaleCanStealMostDelegateRewards() external {
    // 24 tokenholders self delegate their tokens to enable voting on the proposals
    _selfDelegateVoters(_token, _votersArr);

    changePrank(_tokenDeployer);
    _token.transfer(_tokenHolder1, 300_000_000 * 1e18);

    vm.roll(_startBlock + 150);

    // start distribution period
    _startDistributionPeriod(_grantFund);

    uint24 distributionId = _grantFund.getDistributionId();

    (, , , uint128 gbc, , ) = _grantFund.getDistributionPeriodInfo(distributionId);

    assertEq(gbc, 15_000_000 * 1e18);

    TestProposalParams[] memory testProposalParams = new TestProposalParams[](1);
    testProposalParams[0] = TestProposalParams(_tokenHolder1, 8_500_000 * 1e18);

    // create 7 proposals paying out tokens
    TestProposal[] memory testProposals = _createNProposals(_grantFund, _token, testProposalParams);
    assertEq(testProposals.length, 1);

    vm.roll(_startBlock + 200);

    // screening period votes
    _screeningVote(_grantFund, _tokenHolder1, testProposals[0].proposalId, _getScreeningVotes(_grantFund, _tokenHolder1));
    _screeningVote(_grantFund, _tokenHolder2, testProposals[0].proposalId, _getScreeningVotes(_grantFund, _tokenHolder2));

    /*********************/
    /*** Funding Stage ***/
    /*********************/

    // skip time to move from screening period to funding period
    vm.roll(_startBlock + 600_000);


    // check topTenProposals array is correct after screening period - only six should have advanced
    GrantFund.Proposal[] memory screenedProposals = _getProposalListFromProposalIds(_grantFund, _grantFund.getTopTenProposals(distributionId));

    // funding period votes for two competing slates, 1, or 2 and 3
    _fundingVote(_grantFund, _tokenHolder1, screenedProposals[0].proposalId, voteYes, 350_000_000 * 1e18);
    _fundingVote(_grantFund, _tokenHolder2, screenedProposals[0].proposalId, voteYes, 50_000_000 * 1e18);

    /************************/
    /*** Challenge Period ***/
    /************************/

    uint256[] memory potentialProposalSlate = new uint256[](1);
    potentialProposalSlate[0] = screenedProposals[0].proposalId;

    // skip to the end of the DistributionPeriod
    vm.roll(_startBlock + 650_000);

    vm.expectEmit(true, true, false, true);
    emit FundedSlateUpdated(distributionId, _grantFund.getSlateHash(potentialProposalSlate));
    bool proposalSlateUpdated = _grantFund.updateSlate(potentialProposalSlate, distributionId);
    assertTrue(proposalSlateUpdated);

    /********************************/
    /*** Execute Funded Proposals ***/
    /********************************/

    // skip to the end of the Distribution's challenge period
    vm.roll(_startBlock + 700_000);

    // execute funded proposals
    _executeProposal(_grantFund, _token, testProposals[0]);

    /******************************/
    /*** Claim Delegate Rewards ***/
    /******************************/

    assertEq(_grantFund.getDelegateReward(distributionId, _tokenHolder1) / 1e18, 1_470_000);
    assertEq(_grantFund.getDelegateReward(distributionId, _tokenHolder2) / 1e18, 30_000);

    // _tokenHolder1 reward is approx 1_470_000/(1_470_000 + 30_000) ~ 98%

    // linear distribution
    // _tokenHolder1 reward is approx 350/(350 + 50) = 87.5%
}

In this test, _tokenHolder1 has 350/50 = 7 times more tokens and leads to getting 98% of the rewards. Had a linear distribution been used, _tokenHolder1 would have received 87.5%, a fairer number.

In fact, it's even better to use a quadratic voting system, being the rewards the square root of the votes. This would incentivize more delegates and increase decentralization.

Tools Used

Vscode, Foundry

Recommended Mitigation Steps

Use a linear or quadratic delegate reward system.

Assessed type

Other

StandardFunding.sol

tokensRequested could fetch only a storage slot instead of 3 due to getting the whole Proposal struct.

_hasDuplicates(...) could receive proposalIds numerically ordered and check if the next is strictly bigger than the previous instead of double for loop.

fundingVote(...) updates state variables once for every voteParams_, when it could cache it in memory and only update once. For example, currentDistribution_.fundingVotePowerCast += incrementalVotingPowerUsed; is updated for each vote, so if there are 5 votes, it will write to storage 5 times, which could be reduced to only 1 time by caching the variable in memory.

Funding.sol

_insertionSortProposalsByVotes(...) - could be a linked list so the indexes in the array would not need to change. Also, could receive hints to help inserting in the list, making it a O(1) operation.

_validateCallDatas(...) enforces that the targets_ all are ajnaTokenAddress, the values_ are 0 and the selector is that of the `transfer(...). It is overcomplicated and it would be cheaper not to set these parameters and just use them when executing. From my understanding it enables integrating with Tally, but consider changing it in the future.