Foundation contest - rfa's results

Building the new creative economy

General Information

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

Foundation

Findings Distribution

Researcher Performance

Rank: 22/28

Findings: 2

Award: $400.36

🌟 Selected for report: 0

🚀 Solo Findings: 0

Findings Information

Awards

251.1035 USDC - $251.10

Labels

bug
QA (Quality Assurance)

External Links

##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 :

  1. The seller want to sell NFT id 1 for 1 ETH
  2. The seller sign a message to approve the buyer to buy NFT id 1 for 1 ETH
  3. The buyer get the NFT
  4. The buyer sell the NFT id 1 to the seller 1.2 ETH
  5. the seller want to sell the NFT id 1 for 2 ETH
  6. Since the signature that was given by the seller in the 2 step is still valid the buyer can buy the NFT id 1 for 1 ETH
  7. Buyer get the NFT with the old price.

Navigation : https://github.com/code-423n4/2022-02-foundation/blob/main/contracts/mixins/NFTMarketPrivateSale.sol#L162

#0 - HardlyDifficult

2022-03-08T21:03:49Z

  • User might accidentally sent to market: Agree! Thanks for pointing this out. Users often make simple copy paste errors like this. We have added the recommended check.
  • Signature replay on NFTMarketPrivateSale: We have added a mapping to track already accepted terms to avoid any issues with signature replay.

#1 - alcueca

2022-03-17T09:56:17Z

Unadjusted score: 30

Findings Information

🌟 Selected for report: Dravee

Also found by: 0x1f8b, CertoraInc, TerrierLover, csanuragjain, gzeon, iain, kenta, rfa, robee, thankthedark

Labels

bug
G (Gas Optimization)
sponsor disputed

Awards

149.2608 USDC - $149.26

External Links

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:

  • The variables being assigned are immutable.
  • The variables apply to every proxy instance.

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

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter