Malicious user able to start auction to any NFT

[code423n4]

Lines of code

https://github.com/with-backed/papr/blob/9528f2711ff0c1522076b9f93fba13f88d5bd5e6/src/NFTEDA/NFTEDA.sol#L48-L67

Vulnerability details

Impact

Any address can start an auction for an NFT, regardless of whether they are the actual owner of the NFT. This vulnerability could allow an attacker to start auctions for NFTs that they do not own, potentially leading to financial losses for the true owner of the NFT and confusion in the market.

Proof of Concept

Consider the following scenario:

• UserA owns an NFT with ID 123 that is registered on the ERC-721 contract MyNFT
• UserB creates a contract that inherits from NFTEDA and starts an auction for UserA’s NFT with ID 123, setting the starting price to 1 ETH
• UserA tries to purchase her own NFT, but since the auction was started by UserB, she is required to pay the starting price of 1 ETH to UserB in order to get her NFT back
• UserB profits from the sale, even though he is not the actual owner of the NFT

Attacker can exploit it we can deploy the Attacker with the address of a victim contract that owns the NFT and the ID of the NFT we want to start an auction for. Then, we can call the auctionForVictimNFT function to start an auction for the victim's NFT even though we do not own it.

pragma solidity ^0.8.17;

import {NFTEDA} from "./NFTEDA.sol";

contract Attacker is NFTEDA {
    address victimContract;

    uint256 nftID;

    constructor(address _victimContract, uint256 _nftID) {
        victimContract = _victimContract;
        nftID = _nftID;
    }

    function auctionForVictimNFT() public {
        NFTEDA.Auction memory auction;
        auction.auctionAssetID = nftID;
        auction.auctionAssetContract = victimContract;
        auction.nftOwner = address(this);
        auction.perPeriodDecayPercentWad = 0;
        auction.secondsInPeriod = 0;
        auction.startPrice = 0;
        auction.paymentAsset = address(0);
        _startAuction(auction);
    }
}

Recommended Mitigation Steps

Require that the caller of the _startAuction function is the owner of the NFT being auctioned. This can be done by adding a check that compares the caller's address to the owner of the NFT using the ERC-721 ownerOf function.

[c4-judge]

trust1995 marked the issue as unsatisfactory: Invalid