code4rena-2022-08-nounsdao-g01

[G‑01] State checks unnecessarily re-fetch Proposals

보고서

Summary

동일한 storage를 여러번 로드하여 가스를 낭비한다 지적했다. 한 번 읽어온 storage 변수를 쭉 활용하라고 제안했다.

Keyword

gas optimization, storage

Vulnerability

    function state(uint256 proposalId) public view returns (ProposalState) {
        require(proposalCount >= proposalId, 'NounsDAO::state: invalid proposal id');
        Proposal storage proposal = proposals[proposalId];
        if (proposal.vetoed) {
            return ProposalState.Vetoed;
        } else if (proposal.canceled) {
            return ProposalState.Canceled;
        } else if (block.number <= proposal.startBlock) {
            return ProposalState.Pending;
        } else if (block.number <= proposal.endBlock) {
            return ProposalState.Active;
        } else if (proposal.forVotes <= proposal.againstVotes || proposal.forVotes < proposal.quorumVotes) {
            return ProposalState.Defeated;
        } else if (proposal.eta == 0) {
            return ProposalState.Succeeded;
        } else if (proposal.executed) {
            return ProposalState.Executed;
        } else if (block.timestamp >= proposal.eta + timelock.GRACE_PERIOD()) {
            return ProposalState.Expired;
        } else {
            return ProposalState.Queued;
        }
    }

state() 함수는 호출할 때마다 storage 변수를 읽어와 이용한다. state() 를 호출하는 caller 쪽에서는 변수값을 이용하기 위해 또 storage 변수를 읽어온다.

File: /contracts/governance/NounsDAOLogicV2.sol
 
347          require(state(proposalId) != ProposalState.Executed, 'NounsDAO::cancel: cannot cancel executed proposal');
348  
349:         Proposal storage proposal = _proposals[proposalId];
 
286          require(
287              state(proposalId) == ProposalState.Succeeded,
288              'NounsDAO::queue: proposal can only be queued if it is succeeded'
289          );
290:         Proposal storage proposal = _proposals[proposalId];
 
324          require(
325              state(proposalId) == ProposalState.Queued,
326              'NounsDAO::execute: proposal can only be executed if it is queued'
327          );
328:         Proposal storage proposal = _proposals[proposalId];
 
377          require(state(proposalId) != ProposalState.Executed, 'NounsDAO::veto: cannot veto executed proposal');
378  
379:         Proposal storage proposal = _proposals[proposalId];
 
593          require(state(proposalId) == ProposalState.Active, 'NounsDAO::castVoteInternal: voting is closed');
594          require(support <= 2, 'NounsDAO::castVoteInternal: invalid vote type');
595:         Proposal storage proposal = _proposals[proposalId];

어차피 storage를 한번 읽어올 것이라면, 굳이 state() 를 호출해 다시 읽기보다는 한번 읽어들인 storage를 활용하는 쪽이 가스 절약적이다.

Impact

동일한 storage 변수를 여러번 로드하여 가스를 낭비한다.

Mitigation

storage 변수를 한 번만 로드하고, 이를 계속 이용한다.


tags: bughunting, nouns dao, smart contract, solidity, gas optimization, gas, solidity storage, severity gas