Skip to content

Latest commit

 

History

History
73 lines (48 loc) · 3.23 KB

M-02.md

File metadata and controls

73 lines (48 loc) · 3.23 KB

tokenIds is not validated in unwrap function

The function used to unwrap NFTs (convert fractional tokens to NFTs) present in the Pair contract doesn't validate that the given tokenIds are valid according to the intended configuration of the contract.

As opposed to to the wrap function, the unwrap function doesn't call _validateTokenIds to check if the tokenIds are valid and are included in the Merkle tree that defines the tradeable tokens allowed in the Pair:

https://github.com/code-423n4/2022-12-caviar/blob/main/src/Pair.sol#L248-L263

function unwrap(uint256[] calldata tokenIds) public returns (uint256 fractionalTokenAmount) {
    // *** Effects *** //

    // burn fractional tokens from sender
    fractionalTokenAmount = tokenIds.length * ONE;
    _burn(msg.sender, fractionalTokenAmount);

    // *** Interactions *** //

    // transfer nfts to sender
    for (uint256 i = 0; i < tokenIds.length; i++) {
        ERC721(nft).safeTransferFrom(address(this), msg.sender, tokenIds[i]);
    }

    emit Unwrap(tokenIds);
}

Impact

If token validation is enabled for the Pair (merkleRoot != 0) then a user may inadvertently redeem an undesired (lower quality/rareness or cheaper) NFT. For this to work, an attacker would need to directly transfer an NFT (from the same collection, but not necessarily valid in the Pair) and the victim tricked into redeeming their fractional tokens for the invalid token id.

In a similar way, if a user directly transfers an NFT (again from the same collection) by mistake, assuming that NFT has higher quality/rareness than the intended valid token ids in the Pair, then an attacker could use a valid NFT to mint the fractional tokens and the redeem those to remove the higher quality NFT from the Pair.

Proof of Concept

First attack. Assumes victim already deposited a valid NFT in the Pair and has at least 1e18 fractional tokens.

  1. Attacker directly transfers the NFT to the pair (by calling ERC721.transferFrom(attacker, pair, tokenId) and not by using Pair.wrap).
  2. Victim inadvertently calls Pair.unwrap with the attacker's tokenId.

Second attack. Assumes victim transfers a higher quality/rareness token to the Pair.

  1. Victim mistakenly transfers the NFT to the pair (again, by calling ERC721.transferFrom(victim, pair, tokenId)).
  2. Attacker uses a valid NFT (lower quality/cheaper) to mint 1e18 fractional tokens using Pair.wrap.
  3. Attacker calls Pair.unwrap with the victim's tokenId to remove the NFT from the pair.

Recommended Mitigation Steps

Validate tokenIds in the unwrap function by providing the Merkle proofs:

function unwrap(uint256[] calldata tokenIds, bytes32[][] calldata proofs) public returns (uint256 fractionalTokenAmount) {
    // *** Checks *** //

    // check the tokens exist in the merkle root
    _validateTokenIds(tokenIds, proofs);

    // *** Effects *** //

    // burn fractional tokens from sender
    fractionalTokenAmount = tokenIds.length * ONE;
    _burn(msg.sender, fractionalTokenAmount);

    // *** Interactions *** //

    // transfer nfts to sender
    for (uint256 i = 0; i < tokenIds.length; i++) {
        ERC721(nft).safeTransferFrom(address(this), msg.sender, tokenIds[i]);
    }

    emit Unwrap(tokenIds);
}