search

code-423n4 / 2022-05-bunker-findings

1 stars 0 forks source link

Owner `CNft.sol` Can Upgrade The Contract And Drain The Underlying NFT Balance #119

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/CNft.sol#L16 https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/CNft.sol#L274-L282

Vulnerability details

Impact

The CNft.sol contract is deployed to allow users to deposit NFTs and mint ERC1155 tokens. These minted tokens can be deposited in Bunker finance's compound fork to earn yield. Because this contract is upgradeable, the deployer who is assumed to be the Bunker team can upgrade the contract to bypass the current implementation of call() which is restricted to prevent arbitrary calls to the underlying NFT. As a result, if call() is modified to not check that `to != underlying, then the contract owner could transfer out all NFTs.

Recommended Mitigation Steps

Consider removing the contract's upgradeability as it introduces numerous rug vectors to its userbase.

bunkerfinance-dev commented 2 years ago

We don't find any sort of "admin can rug" type of issue as high severity; in the future we will replace the admin with a timelock and/or governance.