code-423n4 / 2022-09-nouns-builder-findings

10 stars 6 forks source link

Ddos in Governor.sol #692

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/governor/Governor.sol#L145 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/governor/Governor.sol#L128 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/governor/Governor.sol#L353 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/governor/Governor.sol#L363

Vulnerability details

Impact

An attacker can cancel proposals.

Proof of Concept

The proposal ID depends on 4 variables:_targets, _values, _calldatas, and descriptionHash. https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/governor/Governor.sol#L145

The Problem is that once a proposal is proposed and the proposal ID is stored, no matter if the proposal is executed or canceled or whatever, that proposal is never going to be proposable again and never going to be executable again. At most, it is executed 1 time.

A malicious attacker having proposalThreshold() of voting power can always front-run a proposal, propose it and then cancel it. That proposal is never going to be proposable again.

https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/governor/Governor.sol#L128

https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/governor/Governor.sol#L353

https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/governor/Governor.sol#L363

Tools Used

Manual review

Recommended Mitigation Steps

Add more parameters to the proposal ID to differentiate two proposals with the same structure but proposed by different persons and you can add a nonce to differentiate the same proposal made by the same person at different times.

GalloDaSballo commented 1 year ago

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