The fork mechanism of Nouns DAO may be completely ineffective or abused, because there is no reasonable limit to the maximum or minimum value of the fork threshold.

[code423n4]

Lines of code

https://github.com/nounsDAO/nouns-monorepo/blob/718211e063d511eeda1084710f6a682955e80dcb/packages/nouns-contracts/contracts/governance/NounsDAOV3Admin.sol#L551-L558 https://github.com/nounsDAO/nouns-monorepo/blob/718211e063d511eeda1084710f6a682955e80dcb/packages/nouns-contracts/contracts/governance/NounsDAOLogicV3.sol#L802-L804

Vulnerability details

Impact

Nouns Fork is a Last-Resort Minority Protection Mechanism, created to protect the minority from the tyranny of the majority. As described in this article: https://mirror.xyz/0x10072dfc23771101dC042fD0014f263316a6E400/iN0FKOn_oYVBzlJkwPwK2mhzaeL8K2-W80u82F7fHj8.

In the initial case, if a quorum of 20% of tokens signals to exit, the fork will succeed, but since the system has no reasonable limit to the maximum or minimum value of the fork threshold, the majority can initiate a new proposal to improve the fork Threshold value, for example: 90% or even 100%.

As a result, those who support the fork can never reach the fork threshold. In this case, the fork of DAO will never happen, and the design mechanism to protect the minority from the tyranny of the majority cannot be achieved.

Proof of Concept

1. In NounsDAOV3Admin.sol, there is the following code: https://github.com/nounsDAO/nouns-monorepo/blob/718211e063d511eeda1084710f6a682955e80dcb/packages/nouns-contracts/contracts/governance/NounsDAOV3Admin.sol#L551-L558

function _setForkThresholdBPS(NounsDAOStorageV3.StorageV3 storage ds, uint256 newForkThresholdBPS)
        external
        onlyAdmin(ds)
    {
        emit ForkThresholdSet(ds.forkThresholdBPS, newForkThresholdBPS);

        ds.forkThresholdBPS = newForkThresholdBPS;
    }

Among them, ds.forkThresholdBPS is the core parameter for calculating whether DAO has reached the fork threshold, and the calculation code for the fork threshold is: (adjustedTotalSupply(ds) * ds.forkThresholdBPS) / 10_000;, if the value of ds.forkThresholdBPS The value is set to 10000 or higher, which can raise the fork threshold to an unreachable height.

2. A majority in the DAO can initiate a proposal to change ds.forkThresholdBPS to 10000 or higher. Because the majority firmly holds the right to cast the majority of votes, even if a few people object, the proposal will still be easily passed and implemented according to the normal process.

Proposal initiation parameter reference:

uint8 numTxs = 10;
address[] memory targets = new address[](numTxs);
uint256[] memory values = new uint256[](numTxs);
string[] memory signatures = new string[](numTxs);
bytes[] memory calldatas = new bytes[](numTxs);

uint256 i = 0;
targets[i] = address(daoProxy);
values[i] = 0;
signatures[i] = '_setForkThresholdBPS(uint256 newForkThresholdBPS)';
calldatas[i] = abi.encode(10000);
uint256 proposalId = daoProxy.propose(targets, values, signatures, calldatas, description);

Tools Used

Visual Studio Code Foundry

Recommended Mitigation Steps

Add two constants in the NounsDAOV3Admin.sol file, such as

uint256 public constant MIN_FORK_THRESHOLD_BPS = 1000; // 10%
uint256 public constant MAX_FORK_THRESHOLD_BPS = 4000; // 40%

Then in the _setForkThresholdBPS function, add the verification of the incoming parameter newForkThresholdBPS, such as the following code:

require(
    newForkThresholdBPS >= MIN_FORK_THRESHOLD_BPS &&
    newForkThresholdBPS <= MAX_FORK_THRESHOLD_BPS,
    'NounsDAO::_setForkThresholdBPS: invalid Threshold bps'
);

Note: MAX_FORK_THRESHOLD_BPS = 4000; only represents personal opinion, and ultimately depends on what percentage of people NounsDao wants to protect. It is recommended that the maximum value should not exceed 4900, which is 49% proportionally.

Why set a minimum value? This is because when ds.forkThresholdBPS is too low (for example: 0), then only 1 token is needed to fork the DAO. Once the fork occurs, it cannot be canceled. During the fork, the original DAO's proposal will not be executed, and malicious actors may use this mechanism to launch a DOS attack on the system's proposal.

Assessed type

Other

[c4-pre-sort]

0xSorryNotSorry marked the issue as primary issue

[eladmallel]

This is by design, not an issue. We want to allow the DAO to set any value it wants to set; at the edges zero means the DAO wants any single forking token to create its own fork DAO, while at the other edge over 10K it means the DAO decides to turn this feature off.

And let's say we added such hard-coded limits, if a majority agrees to go outside those bounds they can simply upgrade the DAO logic to fit their needs.

Not an issue for us.

[c4-sponsor]

eladmallel marked the issue as sponsor disputed

[c4-judge]

gzeon-c4 marked the issue as unsatisfactory: Invalid