Platform: Code4rena
Start Date: 24/02/2022
Pot Size: $75,000 USDC
Total HM: 21
Participants: 28
Period: 7 days
Judge: alcueca
Total Solo HM: 15
Id: 94
League: ETH
Rank: 22/28
Findings: 2
Award: $400.36
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: leastwood
Also found by: 0x1f8b, 0xliumin, 0xwags, CertoraInc, Dravee, IllIllI, Ruhum, TerrierLover, WatchPug, cmichel, csanuragjain, defsec, gzeon, hubble, jayjonah8, kenta, kirk-baird, rfa, robee
251.1035 USDC - $251.10
##Low #Title : User might accidentally sent to market
Explanation : the Market contract preventing user to send eth directly to the contract by adding https://github.com/code-423n4/2022-02-foundation/blob/main/contracts/mixins/NFTMarketCore.sol#L36
, however by calling withdrawFrom() with set the to
parameter to market contract, then the market willingly except eth, since the msg.sender is feth, by adding check with require(to != address(market))
on withdrawFrom(), this can prevent user accidentally send etc to the market contract.
Navigation : https://github.com/code-423n4/2022-02-foundation/blob/main/contracts/FETH.sol#L418
#Title : Signature replay on NFTMarketPrivateSale
Explanation : A seller sign a message when the seller want to approve the buyer to buy the NFT, however there no nonce in place for the digest calculation, therefore in certain condition the buyer might buy the NFT again from the seller.
POC :
Navigation : https://github.com/code-423n4/2022-02-foundation/blob/main/contracts/mixins/NFTMarketPrivateSale.sol#L162
#0 - HardlyDifficult
2022-03-08T21:03:49Z
#1 - alcueca
2022-03-17T09:56:17Z
Unadjusted score: 30
🌟 Selected for report: Dravee
Also found by: 0x1f8b, CertoraInc, TerrierLover, csanuragjain, gzeon, iain, kenta, rfa, robee, thankthedark
149.2608 USDC - $149.26
Gas opt
1# Gas opt on deploying/initializing FNDNFTMarket.sol
https://github.com/code-423n4/2022-02-foundation/blob/main/contracts/FNDNFTMarket.sol#L87-L107
the initialize()
function was intended to change nextAuctionId
= 1:
https://github.com/code-423n4/2022-02-foundation/blob/main/contracts/mixins/NFTMarketAuction.sol#L22-L24
by just doing it on constructor()
and change the storage directly without calling NFTMarketAuction._initializeNFTMarketAuction()
in the
initialize()
can save gas:
constructor( address payable treasury, address feth, address royaltyRegistry, uint256 duration, address marketProxyAddress ) FoundationTreasuryNode(treasury) NFTMarketCore(feth) NFTMarketCreators(royaltyRegistry) NFTMarketReserveAuction(duration) NFTMarketPrivateSale(marketProxyAddress) // solhint-disable-next-line no-empty-blocks { nextAuctionId = 1; }
i recommend to remove initialized()
and _initializeNFTMarketAuction()
function. it can save about 30000 gas (by not running initialize()
)
#0 - HardlyDifficult
2022-03-03T13:05:36Z
The nuance here is that the Market is an upgradeable contract. We do have a kind of confusing mix of initializers and constructors. Using constructors with an upgradeable contract is only viable if the following are true:
In this scenario, the variable is not immutable. If we were to make the suggested change, then it would still have the default value of 0 in the proxy instance we point users to.
#1 - alcueca
2022-03-17T16:05:35Z
Wardens couldn't know how you plan to use this, unless you tell them. Therefore the suggestion is valid. Being a one-off gas saving on a governance function the savings are minimal, though.
Score: 100