Back to sryysryy's contests

Nouns DAO contest - sryysryy's results

A DAO-driven NFT project on Ethereum.

General Information

Platform: Code4rena

Start Date: 22/08/2022

Pot Size: $50,000 USDC

Total HM: 4

Participants: 160

Period: 5 days

Judge: gzeon

Total Solo HM: 2

Id: 155

League: ETH

Nouns DAO

Findings Distribution

Researcher Performance

Rank: 64/160

Findings: 2

Award: $52.11

🌟 Selected for report: 0

🚀 Solo Findings: 0

External Links

Code4rena Contest PageGithub Contest RepositoryGithub Findings RepositoryNouns DAO

Findings Information

🌟 Selected for report: IllIllI

Also found by: 0bi, 0x040, 0x1337, 0x1f8b, 0xDjango, 0xNazgul, 0xNineDec, 0xRajeev, 0xSky, 0xSmartContract, 0xbepresent, 0xkatana, 0xmatt, 8olidity, Aymen0909, Bjorn_bug, Bnke0x0, CertoraInc, Ch_301, Chom, CodingNameKiki, Deivitto, DevABDee, DimitarDimitrov, Dravee, ElKu, Funen, GalloDaSballo, GimelSec, Guardian, Haruxe, JC, JansenC, Jeiwan, JohnSmith, KIntern_NA, Lambda, LeoS, Noah3o6, Olivierdem, R2, RaymondFam, Respx, ReyAdmirado, Rohan16, Rolezn, Ruhum, Saintcode_, Sm4rty, SooYa, Soosh, TomJ, Tomo, Trabajo_de_mates, Waze, _Adam, __141345__, ajtra, android69, asutorufos, auditor0517, berndartmueller, bobirichman, brgltd, c3phas, cRat1st0s, carlitox477, catchup, cccz, csanuragjain, d3e4, delfin454000, dipp, djxploit, durianSausage, erictee, exd0tpy, fatherOfBlocks, gogo, hyh, ladboy233, lukris02, mics, mrpathfindr, natzuu, oyc_109, p_crypt0, pashov, pauliax, pfapostol, prasantgupta52, rajatbeladiya, rbserver, ret2basic, rfa, robee, rokinot, rvierdiiev, sach1r0, saian, seyni, shenwilly, sikorico, simon135, sryysryy, sseefried, throttle, tnevler, tonisives, wagmi, xiaoming90, yixxas, z3s, zkhorse, zzzitron

Awards

35.4484 USDC - $35.45

Labels

bug
QA (Quality Assurance)
edited-by-warden

External Links

Link to GitHub issue

[N-01] NON-LIBRARY/INTERFACE FILES SHOULD USE FIXED COMPILER VERSIONS, NOT FLOATING ONES

There are 7 instances of this issue:

// contracts/base/ERC721Checkpointable.sol
35:  pragma solidity ^0.8.6;

// contracts/base/ERC721Enumerable.sol
28:  pragma solidity ^0.8.0;

// contracts/governance/NounsDAOInterfaces.sol
33:  pragma solidity ^0.8.6;

// contracts/governance/NounsDAOLogicV1.sol
61:  pragma solidity ^0.8.6;

// contracts/governance/NounsDAOLogicV2.sol
53:  pragma solidity ^0.8.6;

// contracts/governance/NounsDAOProxy.sol
36:  pragma solidity ^0.8.6;

https://github.com/code-423n4/2022-08-nounsdao/blob/main/contracts/base/ERC721Checkpointable.sol#L35 https://github.com/code-423n4/2022-08-nounsdao/blob/main/contracts/base/ERC721Enumerable.sol#L28 https://github.com/code-423n4/2022-08-nounsdao/blob/main/contracts/governance/NounsDAOLogicV1.sol#L61 https://github.com/code-423n4/2022-08-nounsdao/blob/main/contracts/governance/NounsDAOInterfaces.sol#L33 https://github.com/code-423n4/2022-08-nounsdao/blob/main/contracts/governance/NounsDAOProxy.sol#L36 https://github.com/code-423n4/2022-08-nounsdao/blob/main/contracts/governance/NounsDAOLogicV2.sol#L53

   

[N-02] USE THE SAME SOLIDITY VERSION

_There is 1 instance of this issue:

// contracts/base/ERC721Enumerable.sol
28: pragma solidity ^0.8.0;

https://github.com/code-423n4/2022-08-nounsdao/blob/main/contracts/base/ERC721Enumerable.sol#L28

   

[N-03] MULTIPLE ADDRESS MAPPINGS CAN BE COMBINED INTO A SINGLE MAPPING OF AN ADDRESS TO A STRUCT, WHERE APPROPRIATE

_There is 1 instance of this issue:

// contracts/base/ERC721Checkpointable.sol
44:    mapping(address => address) private _delegates;

https://github.com/code-423n4/2022-08-nounsdao/blob/main/contracts/base/ERC721Checkpointable.sol#L44

   

[N-04] UNUSED/EMPTY RECEIVE()/FALLBACK() FUNCTION

_There is 1 instance of this issue:

// contracts/governance/NounsDAOLogicV2.sol
1030:    receive() external payable {}

https://github.com/code-423n4/2022-08-nounsdao/blob/main/contracts/governance/NounsDAOLogicV2.sol#L1030      

[L-01] NO REQUIRE CHECKS FOR .CALL

Function may fail silently due to missing require check There are 2 instances of this issue:

// contracts/governance/NounsDAOLogicV2.sol

783:    function _withdraw() external {
784:        if (msg.sender != admin) {
785:            revert AdminOnly();
786:        }
787:        uint256 amount = address(this).balance;
788:        (bool sent, ) = msg.sender.call{ value: amount }('');
789:        emit Withdraw(amount, sent);
790:    }

974:    function _refundGas(uint256 startGas) internal {
975:        unchecked {
976:            uint256 balance = address(this).balance;
977:            if (balance == 0) {
978:                return;
979:            }
980:            uint256 gasPrice = min(tx.gasprice, block.basefee + MAX_REFUND_PRIORITY_FEE);
981:            uint256 gasUsed = startGas - gasleft() + REFUND_BASE_GAS;
982:            uint256 refundAmount = min(gasPrice * gasUsed, balance);
983:            (bool refundSent, ) = msg.sender.call{ value: refundAmount }('');
984:            emit RefundableVote(msg.sender, refundAmount, refundSent);
985:        }
986:    }

https://github.com/code-423n4/2022-08-nounsdao/blob/main/contracts/governance/NounsDAOLogicV2.sol#L783-L790 https://github.com/code-423n4/2022-08-nounsdao/blob/main/contracts/governance/NounsDAOLogicV2.sol#L974-L986

Findings Information

🌟 Selected for report: IllIllI

Also found by: 0x040, 0x1f8b, 0xDjango, 0xNazgul, 0xNineDec, 0xSmartContract, 0xbepresent, 0xc0ffEE, 0xkatana, 2997ms, ACai, Amithuddar, Aymen0909, Ben, BipinSah, Bjorn_bug, Bnke0x0, CertoraInc, Ch_301, Chom, CodingNameKiki, Deivitto, DevABDee, DimitarDimitrov, Diraco, Dravee, ElKu, EthLedger, Fitraldys, Funen, GalloDaSballo, GimelSec, Guardian, IgnacioB, JC, JohnSmith, Junnon, KIntern_NA, Lambda, LeoS, Noah3o6, Olivierdem, Polandia94, R2, Randyyy, RaymondFam, Respx, ReyAdmirado, Rohan16, RoiEvenHaim, Rolezn, Ruhum, SaharAP, Saintcode_, SerMyVillage, Shishigami, Sm4rty, SooYa, TomJ, Tomio, Tomo, Waze, Yiko, _Adam, __141345__, a12jmx, ajtra, ak1, bobirichman, brgltd, bulej93, c3phas, cRat1st0s, carlitox477, catchup, ch0bu, d3e4, delfin454000, djxploit, durianSausage, erictee, exolorkistis, fatherOfBlocks, francoHacker, gogo, hyh, ignacio, jag, joestakey, karanctf, ladboy233, lucacez, lukris02, m_Rassska, martin, medikko, mics, mrpathfindr, natzuu, newfork01, oyc_109, pauliax, peritoflores, pfapostol, prasantgupta52, rbserver, ret2basic, rfa, robee, rokinot, rotcivegaf, rvierdiiev, sach1r0, saian, samruna, seyni, shark, shr1ftyy, sikorico, simon135, sryysryy, tay054, tnevler, wagmi, zishansami

Awards

16.6573 USDC - $16.66

Labels

bug
G (Gas Optimization)

External Links

Link to GitHub issue

[G-01] <ARRAY>.LENGTH SHOULD NOT BE LOOKED UP IN EVERY LOOP OF A FOR-LOOP

There are 8 instances of this issue:

// contracts/governance/NounsDAOLogicV1.sol
281:  for (uint256 i = 0; i < proposal.targets.length; i++) {
319:  for (uint256 i = 0; i < proposal.targets.length; i++) {
346:  for (uint256 i = 0; i < proposal.targets.length; i++) {
371:  for (uint256 i = 0; i < proposal.targets.length; i++) {

// contracts/governance/NounsDAOLogicV2.sol
292:  for (uint256 i = 0; i < proposal.targets.length; i++) {
330:  for (uint256 i = 0; i < proposal.targets.length; i++) {
357:  for (uint256 i = 0; i < proposal.targets.length; i++) {
382:  for (uint256 i = 0; i < proposal.targets.length; i++) {

https://github.com/code-423n4/2022-08-nounsdao/blob/main/contracts/governance/NounsDAOLogicV1.sol#L281 https://github.com/code-423n4/2022-08-nounsdao/blob/main/contracts/governance/NounsDAOLogicV2.sol#L292

   

[G-02] USING PRIVATE RATHER THAN PUBLIC FOR CONSTANTS, SAVES GAS

If needed, the value can be read from the verified contract source code. Savings are due to the compiler not having to create non-payable getter functions for deployment calldata, and not adding another entry to the method ID table

There are 30 instance of this issue:

// contracts/base/ERC721Checkpointable.sol
41:    uint8 public constant decimals = 0;
59:    bytes32 public constant DOMAIN_TYPEHASH =
60:        keccak256('EIP712Domain(string name,uint256 chainId,address verifyingContract)');

// contracts/governance/NounsDAOLogicV1.sol
67:    string public constant name = 'Nouns DAO';
70:    uint256 public constant MIN_PROPOSAL_THRESHOLD_BPS = 1; // 1 basis point or 0.01%
73:    uint256 public constant MAX_PROPOSAL_THRESHOLD_BPS = 1_000; // 1,000 basis points or 10%
76:    uint256 public constant MIN_VOTING_PERIOD = 5_760; // About 24 hours
79:    uint256 public constant MAX_VOTING_PERIOD = 80_640; // About 2 weeks
82:    uint256 public constant MIN_VOTING_DELAY = 1;
85:    uint256 public constant MAX_VOTING_DELAY = 40_320; // About 1 week
88:    uint256 public constant MIN_QUORUM_VOTES_BPS = 200; // 200 basis points or 2%
91:    uint256 public constant MAX_QUORUM_VOTES_BPS = 2_000; // 2,000 basis points or 20%
94:    uint256 public constant proposalMaxOperations = 10; // 10 actions
97:    bytes32 public constant DOMAIN_TYPEHASH =
98:        keccak256('EIP712Domain(string name,uint256 chainId,address verifyingContract)');
101:    bytes32 public constant BALLOT_TYPEHASH = keccak256('Ballot(uint256 proposalId,uint8 support)');
    
// contracts/governance/NounsDAOLogicV2.sol
59:    string public constant name = 'Nouns DAO';
62:    uint256 public constant MIN_PROPOSAL_THRESHOLD_BPS = 1; // 1 basis point or 0.01%
65:    uint256 public constant MAX_PROPOSAL_THRESHOLD_BPS = 1_000; // 1,000 basis points or 10%
68:    uint256 public constant MIN_VOTING_PERIOD = 5_760; // About 24 hours
71:    uint256 public constant MAX_VOTING_PERIOD = 80_640; // About 2 weeks
74:    uint256 public constant MIN_VOTING_DELAY = 1;
77:    uint256 public constant MAX_VOTING_DELAY = 40_320; // About 1 week
80:    uint256 public constant MIN_QUORUM_VOTES_BPS_LOWER_BOUND = 200; // 200 basis points or 2%
83:    uint256 public constant MIN_QUORUM_VOTES_BPS_UPPER_BOUND = 2_000; // 2,000 basis points or 20%
86:    uint256 public constant MAX_QUORUM_VOTES_BPS_UPPER_BOUND = 6_000; // 4,000 basis points or 60%
89:    uint256 public constant MAX_QUORUM_VOTES_BPS = 2_000; // 2,000 basis points or 20%
92:    uint256 public constant proposalMaxOperations = 10; // 10 actions
95:    uint256 public constant MAX_REFUND_PRIORITY_FEE = 2 gwei;
98:    uint256 public constant REFUND_BASE_GAS = 36000;
101:    bytes32 public constant DOMAIN_TYPEHASH =
102:        keccak256('EIP712Domain(string name,uint256 chainId,address verifyingContract)');
105:    bytes32 public constant BALLOT_TYPEHASH = keccak256('Ballot(uint256 proposalId,uint8 support)');

https://github.com/code-423n4/2022-08-nounsdao/blob/main/contracts/base/ERC721Checkpointable.sol#L41 https://github.com/code-423n4/2022-08-nounsdao/blob/main/contracts/governance/NounsDAOLogicV1.sol#L67 https://github.com/code-423n4/2022-08-nounsdao/blob/main/contracts/governance/NounsDAOLogicV2.sol#L59

   

[G-03] IT COSTS MORE GAS TO INITIALIZE VARIABLES TO ZERO THAN TO LET THE DEFAULT OF ZERO BE APPLIED

There are 10 instances of this issue:

// contracts/base/ERC721Checkpointable.sol
181:  uint32 lower = 0;

// contracts/governance/NounsDAOLogicV1.sol
281:  for (uint256 i = 0; i < proposal.targets.length; i++) {
319:  for (uint256 i = 0; i < proposal.targets.length; i++) {
346:  for (uint256 i = 0; i < proposal.targets.length; i++) {
371:  for (uint256 i = 0; i < proposal.targets.length; i++) {

// contracts/governance/NounsDAOLogicV2.sol
292:  for (uint256 i = 0; i < proposal.targets.length; i++) {
330:  for (uint256 i = 0; i < proposal.targets.length; i++) {
357:  for (uint256 i = 0; i < proposal.targets.length; i++) {
382:  for (uint256 i = 0; i < proposal.targets.length; i++) {
948:  uint256 lower = 0;

https://github.com/code-423n4/2022-08-nounsdao/blob/main/contracts/base/ERC721Checkpointable.sol#L181 https://github.com/code-423n4/2022-08-nounsdao/blob/main/contracts/governance/NounsDAOLogicV1.sol#L281 https://github.com/code-423n4/2022-08-nounsdao/blob/main/contracts/governance/NounsDAOLogicV2.sol#L292

   

[G-04] ++I COSTS LESS GAS THAN I++, ESPECIALLY WHEN IT’S USED IN FOR-LOOPS (--I/I-- TOO)

Saves 6 gas PER LOOP

There are 8 instances of this issue:

// contracts/governance/NounsDAOLogicV1.sol
281:  for (uint256 i = 0; i < proposal.targets.length; i++) {
319:  for (uint256 i = 0; i < proposal.targets.length; i++) {
346:  for (uint256 i = 0; i < proposal.targets.length; i++) {
371:  for (uint256 i = 0; i < proposal.targets.length; i++) {

// contracts/governance/NounsDAOLogicV2.sol
292:  for (uint256 i = 0; i < proposal.targets.length; i++) {
330:  for (uint256 i = 0; i < proposal.targets.length; i++) {
357:  for (uint256 i = 0; i < proposal.targets.length; i++) {
382:  for (uint256 i = 0; i < proposal.targets.length; i++) {

https://github.com/code-423n4/2022-08-nounsdao/blob/main/contracts/governance/NounsDAOLogicV1.sol#L281 https://github.com/code-423n4/2022-08-nounsdao/blob/main/contracts/governance/NounsDAOLogicV2.sol#L292

   

[G-05] Use shift operator instead of division

There are 2 instances of this issue:

// contracts/base/ERC721Checkpointable.sol
184:  uint32 center = upper - (upper - lower) / 2; // ceil, avoiding overflow

// contracts/governance/NounsDAOLogicV2.sol
951:  uint256 center = upper - (upper - lower) / 2;

https://github.com/code-423n4/2022-08-nounsdao/blob/main/contracts/base/ERC721Checkpointable.sol#L184 https://github.com/code-423n4/2022-08-nounsdao/blob/main/contracts/governance/NounsDAOLogicV2.sol#L951

   

[G-06] SPLITTING REQUIRE() STATEMENTS THAT USE && SAVES GAS

_There are 19 instances of this issue:


// contracts/governance/NounsDAOLogicV1.sol
126:        require(
127:            votingPeriod_ >= MIN_VOTING_PERIOD && votingPeriod_ <= MAX_VOTING_PERIOD,
128:            'NounsDAO::initialize: invalid voting period'
129:        );

130:        require(
131:            votingDelay_ >= MIN_VOTING_DELAY && votingDelay_ <= MAX_VOTING_DELAY,
132:            'NounsDAO::initialize: invalid voting delay'
133:        );

134:        require(
135:            proposalThresholdBPS_ >= MIN_PROPOSAL_THRESHOLD_BPS && proposalThresholdBPS_ <= MAX_PROPOSAL_THRESHOLD_BPS,
136:            'NounsDAO::initialize: invalid proposal threshold'
137:        );

138:        require(
139:            quorumVotesBPS_ >= MIN_QUORUM_VOTES_BPS && quorumVotesBPS_ <= MAX_QUORUM_VOTES_BPS,
140:            'NounsDAO::initialize: invalid proposal threshold'
141:        );

191:        require(
192:            targets.length == values.length &&
193:                targets.length == signatures.length &&
194:                targets.length == calldatas.length,
195:            'NounsDAO::propose: proposal function information arity mismatch'
196:        );

531:        require(
532:            newVotingDelay >= MIN_VOTING_DELAY && newVotingDelay <= MAX_VOTING_DELAY,
533:            'NounsDAO::_setVotingDelay: invalid voting delay'
534:        );

547:        require(
548:            newVotingPeriod >= MIN_VOTING_PERIOD && newVotingPeriod <= MAX_VOTING_PERIOD,
549:            'NounsDAO::_setVotingPeriod: invalid voting period'
550:        );

564:        require(
565:            newProposalThresholdBPS >= MIN_PROPOSAL_THRESHOLD_BPS &&
566:                newProposalThresholdBPS <= MAX_PROPOSAL_THRESHOLD_BPS,
567:            'NounsDAO::_setProposalThreshold: invalid proposal threshold'
568:        );

582:        require(
583:            newQuorumVotesBPS >= MIN_QUORUM_VOTES_BPS && newQuorumVotesBPS <= MAX_QUORUM_VOTES_BPS,
584:            'NounsDAO::_setProposalThreshold: invalid proposal threshold'
585:        );

617:        require(msg.sender == pendingAdmin && msg.sender != address(0), 'NounsDAO::_acceptAdmin: pending admin only');

// contracts/governance/NounsDAOLogicV2.sol
137:        require(
138:            votingPeriod_ >= MIN_VOTING_PERIOD && votingPeriod_ <= MAX_VOTING_PERIOD,
139:            'NounsDAO::initialize: invalid voting period'
140:        );

141:        require(
142:            votingDelay_ >= MIN_VOTING_DELAY && votingDelay_ <= MAX_VOTING_DELAY,
143:            'NounsDAO::initialize: invalid voting delay'
144:        );
        
145:        require(
146:            proposalThresholdBPS_ >= MIN_PROPOSAL_THRESHOLD_BPS && proposalThresholdBPS_ <= MAX_PROPOSAL_THRESHOLD_BPS,
147:            'NounsDAO::initialize: invalid proposal threshold bps'
148:        );

201:        require(
202:            targets.length == values.length &&
203:                targets.length == signatures.length &&
204:                targets.length == calldatas.length,
205:            'NounsDAO::propose: proposal function information arity mismatch'
206:        );

623:        require(
624:            newVotingDelay >= MIN_VOTING_DELAY && newVotingDelay <= MAX_VOTING_DELAY,
625:            'NounsDAO::_setVotingDelay: invalid voting delay'
626:        );

639:        require(
640:            newVotingPeriod >= MIN_VOTING_PERIOD && newVotingPeriod <= MAX_VOTING_PERIOD,
641:            'NounsDAO::_setVotingPeriod: invalid voting period'
642:        );

656:        require(
657:            newProposalThresholdBPS >= MIN_PROPOSAL_THRESHOLD_BPS &&
658:                newProposalThresholdBPS <= MAX_PROPOSAL_THRESHOLD_BPS,
659:            'NounsDAO::_setProposalThreshold: invalid proposal threshold bps'
660:        );

678:        require(
679:            newMinQuorumVotesBPS >= MIN_QUORUM_VOTES_BPS_LOWER_BOUND &&
680:                newMinQuorumVotesBPS <= MIN_QUORUM_VOTES_BPS_UPPER_BOUND,
681:            'NounsDAO::_setMinQuorumVotesBPS: invalid min quorum votes bps'
682:        );

819:        require(msg.sender == pendingAdmin && msg.sender != address(0), 'NounsDAO::_acceptAdmin: pending admin only');

https://github.com/code-423n4/2022-08-nounsdao/blob/main/contracts/governance/NounsDAOLogicV1.sol#L126-L129 https://github.com/code-423n4/2022-08-nounsdao/blob/main/contracts/governance/NounsDAOLogicV2.sol#L137-L140

   

[G-07] USE A MORE RECENT VERSION OF SOLIDITY

Use a solidity version of at least 0.8.2 to get simple compiler automatic inlining Use a solidity version of at least 0.8.3 to get better struct packing and cheaper multiple storage reads Use a solidity version of at least 0.8.4 to get custom errors, which are cheaper at deployment than revert()/require() strings Use a solidity version of at least 0.8.10 to have external calls skip contract existence checks if the external call has a return value

_There is 1 instance of this issue:

// contracts/base/ERC721Enumerable.sol
28: pragma solidity ^0.8.0;

https://github.com/code-423n4/2022-08-nounsdao/blob/main/contracts/base/ERC721Enumerable.sol#L28

   

[G-08] DON’T COMPARE BOOLEAN EXPRESSIONS TO BOOLEAN LITERALS

if (<x> == true) => if (<x>), if (<x> == false) => if (!<x>)

_There are 2 instances of this issue:

// contracts/governance/NounsDAOLogicV1.sol
505:        require(receipt.hasVoted == false, 'NounsDAO::castVoteInternal: voter already voted');


// contracts/governance/NounsDAOLogicV2.sol
597:        require(receipt.hasVoted == false, 'NounsDAO::castVoteInternal: voter already voted');

https://github.com/code-423n4/2022-08-nounsdao/blob/main/contracts/base/ERC721Checkpointable.sol#L505 https://github.com/code-423n4/2022-08-nounsdao/blob/main/contracts/governance/NounsDAOLogicV2.sol#L597

[G-09] ++I/I++ SHOULD BE UNCHECKED{++I}/UNCHECKED{I++} WHEN IT IS NOT POSSIBLE FOR THEM TO OVERFLOW, AS IS THE CASE WHEN USED IN FOR- AND WHILE-LOOPS

There are 8 instances of this issue:

// contracts/governance/NounsDAOLogicV1.sol
281:  for (uint256 i = 0; i < proposal.targets.length; i++) {
319:  for (uint256 i = 0; i < proposal.targets.length; i++) {
346:  for (uint256 i = 0; i < proposal.targets.length; i++) {
371:  for (uint256 i = 0; i < proposal.targets.length; i++) {

// contracts/governance/NounsDAOLogicV2.sol
292:  for (uint256 i = 0; i < proposal.targets.length; i++) {
330:  for (uint256 i = 0; i < proposal.targets.length; i++) {
357:  for (uint256 i = 0; i < proposal.targets.length; i++) {
382:  for (uint256 i = 0; i < proposal.targets.length; i++) {

https://github.com/code-423n4/2022-08-nounsdao/blob/main/contracts/governance/NounsDAOLogicV1.sol#L281 https://github.com/code-423n4/2022-08-nounsdao/blob/main/contracts/governance/NounsDAOLogicV2.sol#L292

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter