Nouns Builder contest - csanuragjain's results

Lines of code

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

Vulnerability details

Impact

Defeated proposal will be mark accepted and would be executed due to an incorrect condition.

Proof of Concept

1. Proposal A was created
2. proposal.voteEnd has reached and no one voted on this proposal

proposal.forVotes = 0
proposal.againstVotes = 0

3. As checked with product team, in case of tie proposal should get Defeated

4. Due to an incorrect condition (missing =), it will instead be Queued for execution, since proposal.forVotes < proposal.againstVotes will be false

function state(bytes32 _proposalId) public view returns (ProposalState) {
...
else if (proposal.forVotes < proposal.againstVotes || proposal.forVotes < proposal.quorumVotes) {
            return ProposalState.Defeated;
}
...
else {
            return ProposalState.Queued;
        }
}

Recommended Mitigation Steps

Change the condition to include = while comparing votes as shown below

else if (proposal.forVotes <= proposal.againstVotes || proposal.forVotes < proposal.quorumVotes) {
            return ProposalState.Defeated;
}

User can overwrite other user bids

Contract: https://github.com/code-423n4/2022-09-nouns-builder/blob/main/src/auction/Auction.sol#L331

Impact

If admin sets minimumBidIncrement to 0 percent then one User can overwrite other user bids without need to provide extra funds

Proof of Concept

1. Assume that settings.minBidIncrement is currently set to 0 by Admin
2. User A creates bid on the auction with amount X
3. Since settings.minBidIncrement is 0, so User B only needs to provide amount X which will replace User A bid with User B bid

Recommended Mitigation Steps

Add a check to make sure settings.minBidIncrement>0

require(settings.minBidIncrement>0, "Incorrect increment");

Auction can be extended to an indefinite amount of time

Contract: https://github.com/code-423n4/2022-09-nouns-builder/blob/main/src/auction/Auction.sol#L323

Impact

Auction can be extended to an indefinite amount of time

Proof of Concept

1. Assume that settings.timeBuffer is currently set to a 1000 days by Admin
2. User A creates bid on the auction with amount X
3. Since settings.timeBuffer is 1000 days, so new Auction can only be created post 1000 days

Recommended Mitigation Steps

There should be a max cap on timeBuffer