Nouns Builder contest - immeas's results

Lines of code

https://github.com/code-423n4/2022-09-nouns-builder/blob/main/src/token/Token.sol#L157

Vulnerability details

Impact

Infinite loop consuming all gas in Token.sol when founders have 100% ownership. 100% ownership is the max allowed ownership. If DAO is setup with this ownership the Token contract is unusable and must scrapped, either by an upgrade or a complete redploy of the whole DAO.

It's not impossible to think of a use case for 100% ownership. For example: all the tokens during a vesting period goes to founders and after vesting period ends founders hold a certain amount of tokens which is constant. Then founder impact decreases over time as more and more tokens become available.

Proof of Concept

In use by auction:

diff --git a/test/utils/NounsBuilderTest.sol b/test/utils/NounsBuilderTest.sol
index cb17d6b..fc8b21c 100644
--- a/test/utils/NounsBuilderTest.sol
+++ b/test/utils/NounsBuilderTest.sol
@@ -89,8 +89,8 @@ contract NounsBuilderTest is Test {
         wallets[0] = founder;
         wallets[1] = founder2;
 
-        percents[0] = 10;
-        percents[1] = 5;
+        percents[0] = 50;
+        percents[1] = 50;
 
         vestingEnds[0] = 4 weeks;
         vestingEnds[1] = 4 weeks;

Then forge test --match-test=test_Unpause test will fail due to all gas expended

or directly on the Token contract:

diff --git a/test/Token.t.sol b/test/Token.t.sol
index 08eadd1..ee27480 100644
--- a/test/Token.t.sol
+++ b/test/Token.t.sol
@@ -188,6 +188,30 @@ contract TokenTest is NounsBuilderTest, TokenTypesV1 {
         }
     }
 
+    function test_MaxOwnershipMintToken() public {
+        createUsers(2, 1 ether);
+
+        address[] memory wallets = new address[](2);
+        uint256[] memory percents = new uint256[](2);
+        uint256[] memory vestExpirys = new uint256[](2);
+
+        uint256 pct = 50;
+        uint256 end = 4 weeks;
+
+        unchecked {
+            for (uint256 i; i < 2; ++i) {
+                wallets[i] = otherUsers[i];
+                percents[i] = pct;
+                vestExpirys[i] = end;
+            }
+        }
+
+        deployWithCustomFounders(wallets, percents, vestExpirys);
+
+        vm.prank(address(auction));
+        token.mint(); // will infinitely loop 
+    }
+
     function testRevert_OnlyAuctionCanMint() public {
         deployMock();

Tools Used

forge, vscode

Recommended Mitigation Steps

Either stop allowing 100% ownership or introduce a max tokens that can be minted in one go. Depends on the behavior you want when 100% ownership.