Snapshot of user's balance can be manipulated

[code423n4]

Lines of code

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

Vulnerability details

Impact

A malicious user is able to vote with more vote than he actually has thus having more say in proposals where his manipulated balance is valid.

Description

This attack vector is possible because ERC721Checkpointable.getPriorVotes() looks at a snapshot of a user's balance at a particular block. A malicious user can borrow Nouns from a lending platform (e.g. NFTx, NFTfi) to inflate their Nouns balance before a proposal is created. Once the proposal is created, they will return the borrowed asset.

When the proposal is becomes active and castVote() is called, ERC721Checkpointable.getPriorVotes() will look at the user's inflated Nouns balance before the proposal was created thereby giving them more influence over the voting.

Proof of Concept

• Discussion takes place on the forum
• A user with vested interest to pass the proposal will borrow Nouns before this proposal is created
• The Proposal is created
• The user then returns borrowed assets
• When the proposal becomes active, the user will have inflated voting rights because of the snapshot.

Tools Used

Manual analysis

Recommended Mitigation Steps

• Since checkpoints are already used in beforeTokenTransfer, it should be possible to also introduce a time weighted average balance (similar to that of Uniswap's TWAP) to be used specifically for voting. This lag will make it harder / more expensive to perform this snapshot attack and because this "weighted balanceOf", it does not have an impact on the standard functionality of an ERC721 token.

[eladmallel]

Duplicate of #140