code-423n4 / 2022-08-foundation-findings

0 stars 0 forks source link

users could mint NFTs for free #218

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-08-foundation/blob/main/contracts/mixins/nftDropMarket/NFTDropMarketFixedPriceSale.sol#L170-L207 https://github.com/code-423n4/2022-08-foundation/blob/main/contracts/mixins/shared/FETHNode.sol#L46-L55 https://github.com/code-423n4/2022-08-foundation/blob/main/contracts/FETH.sol#L343-L348

Vulnerability details

Impact

Malicious users could mint NFTs AMAP by the collection saleConfig.limitPerAccount with msg.value == 0 ETH

Proof of Concept

The malicious users call mintFromFixedPriceSale() with 0 ETH. the only check is if (msg.value > mintCost)and there is no check for msg.value < mintCost and msg.value == 0. They need to deposit some ETH in their account and convert it to FETH. So when mintFromFixedPriceSale() call _tryUseFETHBalance() they can bypass the check-in _deductBalanceFrom() it call by marketWithdrawFrom() after that they get back there FETH by this line payable(msg.sender).sendValue(amount);

There is the possibility of a Re-entrancy attack if the creator sets a high value for saleConfig.limitPerAccount

Recommended Mitigation Steps

Add check revert for msg.value < mintCost and msg.value == 0

HardlyDifficult commented 2 years ago

Invalid as described. When marketWithdrawFrom is called, since that is an external contract the msg.sender receiving funds is the drop market contract address and not the user.

HickupHH3 commented 2 years ago

Agree with sponsor's reasoning.