Digital garden

code4rena-2022-08-nounsdao-n07

3 min read

code4rena-2022-08-nounsdao-n07

[N‑07] Constant redefined elsewhere

보고서

Summary

동일한 constant를 여러 컨트랙트에서 중복하여 정의하면 추후 값이 바뀌어야 할 때 모든 컨트랙트에서 수정하는 것을 까먹어 싱크가 안 맞는 상황이 생길 수 있다. 따라서 constant는 한 군데에서만 정의하는 게 좋다고 제안했다.

Keyword

clean code

Vulnerability

File: contracts/governance/NounsDAOLogicV2.sol
 
/// @audit seen in contracts/governance/NounsDAOLogicV1.sol 
59:       string public constant name = 'Nouns DAO';
 
/// @audit seen in contracts/governance/NounsDAOLogicV1.sol 
62:       uint256 public constant MIN_PROPOSAL_THRESHOLD_BPS = 1; // 1 basis point or 0.01%
 
/// @audit seen in contracts/governance/NounsDAOLogicV1.sol 
65:       uint256 public constant MAX_PROPOSAL_THRESHOLD_BPS = 1_000; // 1,000 basis points or 10%
 
/// @audit seen in contracts/governance/NounsDAOLogicV1.sol 
68:       uint256 public constant MIN_VOTING_PERIOD = 5_760; // About 24 hours
 
/// @audit seen in contracts/governance/NounsDAOLogicV1.sol 
71:       uint256 public constant MAX_VOTING_PERIOD = 80_640; // About 2 weeks
 
/// @audit seen in contracts/governance/NounsDAOLogicV1.sol 
74:       uint256 public constant MIN_VOTING_DELAY = 1;
 
/// @audit seen in contracts/governance/NounsDAOLogicV1.sol 
77:       uint256 public constant MAX_VOTING_DELAY = 40_320; // About 1 week
 
/// @audit seen in contracts/governance/NounsDAOLogicV1.sol 
89:       uint256 public constant MAX_QUORUM_VOTES_BPS = 2_000; // 2,000 basis points or 20%
 
/// @audit seen in contracts/governance/NounsDAOLogicV1.sol 
92:       uint256 public constant proposalMaxOperations = 10; // 10 actions
 
/// @audit seen in contracts/governance/NounsDAOLogicV1.sol 
101       bytes32 public constant DOMAIN_TYPEHASH =
102:          keccak256('EIP712Domain(string name,uint256 chainId,address verifyingContract)');
 
/// @audit seen in contracts/governance/NounsDAOLogicV1.sol 
105:      bytes32 public constant BALLOT_TYPEHASH = keccak256('Ballot(uint256 proposalId,uint8 support)');

동일한 constant를 여러 컨트랙트에서 중복하여 정의하면 추후 값이 바뀌어야 할 때 모든 컨트랙트에서 수정하는 것을 까먹어 싱크가 안 맞는 상황이 생길 수 있다. 따라서 constant는 한 군데에서만 정의하는 게 좋다고 제안했다.

Impact

constant를 수정하고자 할 때, 까먹고 다른 컨트랙트는 수정하지 않는 상황이 발생할 수 있다.

Mitigation

라이브러리를 이용하여 한번만 정의한 후 공유해 사용하는 방법을 추천하였다. library 이용 트릭을 참고하면 library의 internal 함수로 작성하면 낮은 가스로 접근하면서 constant 관련 코드를 따로 관리할 수 있다.

Memo

별걸 다… library 를 이용해도 메리트가 있는지는 잘 모르겠다.

tags: bughunting, nouns dao, clean code, smart contract, solidity, severity none

Graph View