search

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

10 stars 6 forks source link

Founders can re-gain the ownership of `auction` and `MetadataRenderer`. #652

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/lib/utils/Ownable.sol#L63 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/lib/utils/Ownable.sol#L71-L84

Vulnerability details

Impact

founder can re-gain the ownership of auction and MetadataRenderer.

Specifically, he can first invoke Ownable::safeTransferOwnership to make his another wallet as _pendingOwner.

After that, he invokes MetadataRenderer::addProperties or Auction::unpause to transfer the ownership to treasury.

And at any time he wants (or immediately), he can gain the ownership back by calling Ownable::acceptOwnership.

It is critical since he can change properties any time (impact the value of NFT), or pause auction any time (lock users' funds).

It is against the purpose of a DAO.

Recommended Mitigation Steps

Delete the _pendingOwner when transferOwnership.

GalloDaSballo commented 2 years ago

Dup of #414

GalloDaSballo commented 2 years ago

L