code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-05-particle/blob/bbd1c01407a017046c86fdb483bbabfb1fb085d8/contracts/protocol/ParticleExchange.sol#L434-L463 https://github.com/code-423n4/2023-05-particle/blob/bbd1c01407a017046c86fdb483bbabfb1fb085d8/contracts/protocol/ParticleExchange.sol#L47-L60 https://github.com/code-423n4/2023-05-particle/blob/bbd1c01407a017046c86fdb483bbabfb1fb085d8/contracts/protocol/ParticleExchange.sol#L95-L127 https://github.com/code-423n4/2023-05-particle/blob/bbd1c01407a017046c86fdb483bbabfb1fb085d8/contracts/protocol/ParticleExchange.sol#L172-L189

Vulnerability details

Impact

After a borrower calls the following ParticleExchange.swapWithEth function, the borrower receives the corresponding NFT.

https://github.com/code-423n4/2023-05-particle/blob/bbd1c01407a017046c86fdb483bbabfb1fb085d8/contracts/protocol/ParticleExchange.sol#L434-L463

    function swapWithEth(Lien calldata lien, uint256 lienId) external payable override validateLien(lien, lienId) {
        ...

        /// @dev cannot instantly become insolvent (will revert if credit < 0)
        uint256 credit = msg.value - lien.price;

        // update lien
        liens[lienId] = keccak256(
            abi.encode(
                Lien({
                    lender: lien.lender,
                    borrower: msg.sender,
                    collection: lien.collection,
                    tokenId: lien.tokenId,
                    price: lien.price,
                    rate: lien.rate,
                    loanStartTime: block.timestamp,
                    credit: credit,
                    auctionStartTime: 0
                })
            )
        );

        // transfer NFT to borrower
        IERC721(lien.collection).safeTransferFrom(address(this), msg.sender, lien.tokenId);
        ...
    }

It is possible that the borrower calls the following ParticleExchange.supplyNft function to supply the borrowed NFT to the Particle Exchange. For example, if the Particle Exchange has no other NFTs of the corresponding collection after the borrowed NFT was transferred to the borrower, the borrower could supply the borrowed NFT and set the new lien's price and rate to be higher than the original lien's price and rate as an attempt for making a profit.

https://github.com/code-423n4/2023-05-particle/blob/bbd1c01407a017046c86fdb483bbabfb1fb085d8/contracts/protocol/ParticleExchange.sol#L47-L60

    function supplyNft(
        address collection,
        uint256 tokenId,
        uint256 price,
        uint256 rate
    ) external override returns (uint256 lienId) {
        lienId = _supplyNft(msg.sender, collection, tokenId, price, rate);

        // transfer NFT into contract
        /// @dev collection.setApprovalForAll should have been called by this point
        IERC721(collection).safeTransferFrom(msg.sender, address(this), tokenId);

        return lienId;
    }

https://github.com/code-423n4/2023-05-particle/blob/bbd1c01407a017046c86fdb483bbabfb1fb085d8/contracts/protocol/ParticleExchange.sol#L95-L127

    function _supplyNft(
        address lender,
        address collection,
        uint256 tokenId,
        uint256 price,
        uint256 rate
    ) internal returns (uint256 lienId) {
        ...

        // create a new lien
        Lien memory lien = Lien({
            lender: lender,
            borrower: address(0),
            collection: collection,
            tokenId: tokenId,
            price: price,
            rate: rate,
            loanStartTime: 0,
            credit: 0,
            auctionStartTime: 0
        });

        /// @dev Safety: lienId unlikely to overflow by linear increment
        unchecked {
            liens[lienId = _nextLienId++] = keccak256(abi.encode(lien));
        }

        ...
    }

When this occurs, the borrowed NFT is back in the Particle Exchange, and the original lien's lender can call the ParticleExchange.withdrawNftWithInterest function to withdraw such NFT before such NFT is sold. Because lien.tokenId of the original lien is also the token ID of such NFT, calling IERC721(lien.collection).safeTransferFrom(address(this), msg.sender, lien.tokenId) would go through, which causes the borrower to lose such NFT that it supplies for the new lien. Moreover, after liens[lienId] is deleted, the original lien is removed so the borrower cannot get back any of the msg.value that it sent for calling the ParticleExchange.swapWithEth function even though its position for the original lien is not insolvent yet.

https://github.com/code-423n4/2023-05-particle/blob/bbd1c01407a017046c86fdb483bbabfb1fb085d8/contracts/protocol/ParticleExchange.sol#L172-L189

    function withdrawNftWithInterest(Lien calldata lien, uint256 lienId) external override validateLien(lien, lienId) {
        if (msg.sender != lien.lender) {
            revert Errors.Unauthorized();
        }

        // delete lien
        delete liens[lienId];

        // transfer NFT back to lender
        /// @dev can withdraw means NFT is currently in contract without active loan,
        /// @dev the interest (if any) is already accured to lender at NFT acquiring time
        IERC721(lien.collection).safeTransferFrom(address(this), msg.sender, lien.tokenId);

        emit WithdrawNFT(lienId);

        // withdraw interest from this account too
        _withdrawAccountInterest(payable(msg.sender));
    }

Proof of Concept

The following steps can occur for the described scenario.

Alice calls the ParticleExchange.swapWithEth function to borrow Milady 8621. For this lien, Bob is the lender.
Since the Particle Exchange has no other Milady NFTs at this moment, Alice calls the ParticleExchange.supplyNft function to supply Milady 8621 and set the new lien's price and rate to be higher than the original lien's price and rate as an attempt for making a profit.
Bob calls the ParticleExchange.withdrawNftWithInterest function to withdraw Milady 8621, which causes Alice to lose Milady 8621.
Because the original lien is removed, Alice cannot get back any of the msg.value she sent for calling the ParticleExchange.swapWithEth function even though her position for the original lien is not insolvent.

Tools Used

VSCode

Recommended Mitigation Steps

The ParticleExchange.withdrawNftWithInterest function can be updated to revert if lien.loanStartTime != 0 is true.

Assessed type

Token-Transfer

c4-judge commented 1 year ago

hansfriese marked the issue as satisfactory

c4-judge commented 1 year ago

hansfriese marked the issue as duplicate of #13

c4-sponsor commented 1 year ago

wukong-particle marked the issue as sponsor confirmed

wukong-particle commented 1 year ago

Judge is correct, indeed duplication