sherlock-audit / 2023-02-fair-funding-judging

1 stars 0 forks source link

0x52 - It is impossible to change ownership of the MintableERC721 if the AuctionHouse contract needs to be replaced #85

Closed github-actions[bot] closed 1 year ago

github-actions[bot] commented 1 year ago

0x52

medium

It is impossible to change ownership of the MintableERC721 if the AuctionHouse contract needs to be replaced

Summary

MintableERC721 requires that AuctionHouse is the owner of the contract in order to mint tokens. In the event that AuctionHouse needs to be changed or becomes broken/buggy. There is no way to transfer ownership to a new contract to allow continued minting.

Vulnerability Detail

See summary.

Impact

Impossible to change minter of MintableERC721

Code Snippet

https://github.com/sherlock-audit/2023-02-fair-funding/blob/main/fair-funding/contracts/solidity/MintableERC721.sol#L16-L18

Tool used

Manual Review

Recommendation

I see two solutions: 1) Add a function to AuctionHouse that allows it to transfer the ownership of the NFT 2) Add a minter role to MintableERC721 and use that instead of ownerOnly minting

Unstoppable-DeFi commented 1 year ago

Conceptually the auction phase of a Fair Funding campaign will be limited and rather short (for example first Fair Funding campaign will run 16 days / 16 auctions). Compared to the available APYs on Alchemix (2-5% max), the discrepancy between early depositors and late depositors is effectively near zero and negligible.

AuctionHouse and corresponding NFT are distinct and should not be reused.

hrishibhat commented 1 year ago

Closing based on the above comment