code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/mixins/nftDropMarket/NFTDropMarketFixedPriceSale.sol#L170 https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/mixins/shared/MarketFees.sol#L522

Vulnerability details

Impact

Detailed description of the impact of this finding.

When minting,

we go through

  function mintFromFixedPriceSale(
    address nftContract,
    uint16 count,
    address payable buyReferrer
  ) external payable returns (uint256 firstTokenId) {

buyRefererer can be set to arbitrary address to the referral fee because when the referral fee is calcuulated.

    if (buyReferrer != address(0) && buyReferrer != msg.sender && buyReferrer != seller && buyReferrer != creator) {
      unchecked {
        buyReferrerFee = totalFees / BUY_REFERRER_PROTOCOL_FEE_DENOMINATOR;

        // buyReferrerFee is always <= totalFees
        totalFees -= buyReferrerFee;
      }
    }

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

A user Bob wants to mint nft, he set Alice to receive the referral fee, but alice is bob's wallet address.

Tools Used

Recommended Mitigation Steps

We can use another map to keep track of the referral.

 mapping(address => address) referralMap;

and the function

  function refer(address referTo) {
    referralMap[referTo] = msg.sender;
  }

and when a user is minting,

we can check if the referral address is correct.

  function mintFromFixedPriceSale(
    address nftContract,
    uint16 count,
    address payable buyReferrer
  ) external payable returns (uint256 firstTokenId) {

  require(referralMap[msg.sender] == buyReferral, 'invalid referral')

  // other logic

}

ghost commented 2 years ago

Mitigation step does not actually mitigate sybil attack via referral fee since Bob can still set Alice address as a referral and he owns Alice's private key.

HardlyDifficult commented 2 years ago

Dupe of https://github.com/code-423n4/2022-08-foundation-findings/issues/134

HickupHH3 commented 2 years ago

primary warden's QA report

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