search

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

1 stars 0 forks source link

NFTX Vault Owners Can Liquidate Users By Updating `mintFee()` #115

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/Oracles/CNftPriceOracle.sol#L81-L100

Vulnerability details

Impact

The mintFee() value returned by the NFTX vault is used in determining the underlying price of an NFT, mainly because NFTX is the only protocol providing on-chain liquidity for NFTs. The price of an NFT is used to determine how healthy a borrow position, where liquidators will liquidate a position when this value is insufficient. There is a certain degree of centralisation here and I think it would be safer to have this excluded from calculations related to the health of a borrow position. Liquidators should be verifying this data off-chain, as this allows them to use alternative NFT marketplaces (on-chain or off-chain). The mint fee of an NFTX vault can be updated at any point in time by the vault's owner (who is not affliated with Bunker protocol). This mint fee can be increased to charge up to 50% on mints which is almost certain to cause large scale liquidation events within Bunker's protocol.

Recommended Mitigation Steps

Consider removing INFTXVault(nftxToken).mintFee() from the CNftPriceOracle.getUnderlyingPrice() function and instead have users determine the viability of a liquidation off-chain.

bunkerfinance-dev commented 2 years ago

The mint fee can only be changed by governance, so we consider this to be lower severity.

gzeoneth commented 2 years ago

Downgrading to Low / QA.

gzeoneth commented 2 years ago

Consider with warden's QA report #111