search

code-423n4 / 2022-06-nibbl-findings

1 stars 0 forks source link

ETH accumulated by underlying ERC721 in vault from royalties or airdrops are paid out to fictionalized ERC20 holders #88

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/NibblVault.sol#L466

Vulnerability details

All other potential airdrops and royalties (ERC20, ERC721, ERC1155) are given to the bidder so it only seems fair that ETH received in this manner should be given to the bidder as well

Impact

ETH accumulated by underlying ERC721 in vault from royalties or airdrops are paid out to fictionalized ERC20 holders on buyout instead of bidder

Proof of Concept

Redeem gives fictionalized ERC20 holders their proportion of the ETH in the contract less the curator fee and unsettle bid amount. This does not account for any ETH received by the NFT such as royalties and airdrops while the NFT is in the contract. This means that any ETH received in this manner will instead be given to fictionalized ERC20 holders on buyout instead of the bidder.

Tools Used

Recommended Mitigation Steps

Add a fallback function that counts and stores all ETH received from calls with no data as ETH received by the NFT. Add another function that allows the bidder to withdraw this amount after buyout is successful

HardlyDifficult commented 2 years ago

This seems like a consideration aiming to improve the design. It's not clear that this would be the better solution and it does not seem to break the protocol -- so lowering the risk and converting this into a QA report for the warden.