Real-world smart contracts often make external calls to other smart contracts. A smart contract auditor or attacker must always examine these external calls to see if there is a hook that will allow them to re-enter the target contract mid-transaction and exploit it.

One particular red flag to look for is code that executes verification or checks after an external call; can the attacker hook the external call to hijack transaction flow and perform an attack that will after it has concluded pass those subsequent checks, which would not pass if execution had continued normally?

Auditors & attacks should pay special attention to external calls made to ERC standards eg: ERC1155._mintBatch() or ERC721.safeTransferFrom(), which a contract developer may not realize can be hooked by an attacking contract via their callback mechanisms.

Re-entrancy Bypass Verification Checks

The exploit comes from the Papr Code4rena contest. PaprController.sol allows users to deposit NFTs as collateral & borrow Papr tokens. Users can add & remove collateral, and if the value of their collateral falls below a given threshold compared to what they have borrowed, other users are able to trigger a liquidation auction on their collateral.

The following function calls ERC721.SafeTransferFrom() to return an NFT to its owner, then checks if this would exceed the maximum debt allowed due to reduced collateral for the user. An attacker can take control of the execution flow by using an attack contract as ERC721.SafeTransferFrom() calls sendTo.onERC721Received() if sendTo is a contract.

This hook returns execution flow to an attacker before the max debt check is performed, allowing the attacker to re-enter the contract and perform other operations which when complete can satisfy the maximum debt checks and make the attacker a handsome profit.

function _removeCollateral(address sendTo, IPaprController.Collateral calldata collateral,
                           uint256 oraclePrice,uint256 cachedTarget) internal {
    if (collateralOwner[collateral.addr][collateral.id] != msg.sender) {
        revert IPaprController.OnlyCollateralOwner();
    }

    delete collateralOwner[collateral.addr][collateral.id];

    uint16 newCount;
    unchecked {
        newCount = _vaultInfo[msg.sender][collateral.addr].count - 1;
        _vaultInfo[msg.sender][collateral.addr].count = newCount;
    }
    //
    // @audit CRITICAL Re-Entrancy attack due to not following Check-Effects-Interaction pattern
    // ERC721.safeTransferFrom() calls sendTo.onERC721Received() if sendTo is a contract.
    //
    // 1) attacker deposits multiple nfts as collateral into one vault & borrows max amount against them
    //
    // 2) attacker calls removeCollateral() for first nft, then in AttackContract.onERC721Received() 
    //    calls removeCollateral() for second nft & so on until only 1 nft left in vault as collateral
    //
    // 3) for last nft AttackContract.onERC721Received() calls startLiquidationAuction()
    //    then purchaseLiquidationAuctionNFT() to buy their own nft via liquidation auction.
    //    As this was the vault's last collateral nft, the vault debt will be set to 0 which passes the
    //    subsequent checks that would have never passed.
    //
    collateral.addr.safeTransferFrom(address(this), sendTo, collateral.id);

    uint256 debt = _vaultInfo[msg.sender][collateral.addr].debt;
    uint256 max = _maxDebt(oraclePrice * newCount, cachedTarget);

    if (debt > max) {
        revert IPaprController.ExceedsMaxDebt(debt, max);
    }

    emit RemoveCollateral(msg.sender, collateral.addr, collateral.id);
}

Verification checks should always be performed before calling external functions that could be hooked by an attacker to hijack the execution flow and re-enter the contract.

Re-entrancy Bypass Storage Writes

Another red flag to look for is writes to storage that occur after calls to external functions - can you hijack execution flow by hooking the external call and re-enter the contract, taking advantage of the fact that writes to storage have not yet occurred?

The next exploit comes from Pashov's audit of Hypercerts. HypercertMinter.splitValue() allows a claim token to be split into fractions. This function calls SemiFungible1155._splitValue() which calls ERC1155._mintBatch() before writing the decreased valueLeft to storage.

An attack contract can hook execution flow as ERC1155._mintBatch() calls _account.onERC1155BatchReceived() if _account is a contract; the attacker can then call HypercertMinter.splitValue() to split the same tokenId many times to create a huge amount of fractions using the same tokenId.

    /// @dev Split the units of `_tokenID` owned by `account` across `_values`
    /// @dev `_values` must sum to total `units` held at `_tokenID`
    function _splitValue(address _account, uint256 _tokenID, uint256[] calldata _values) internal {
        // ... //
        uint256 valueLeft = tokenValues[_tokenID];
        // ... //

        for (uint256 i; i < len; ) {
            valueLeft -= values[i];

            tokenValues[toIDs[i]] = values[i];

            unchecked {
                ++i;
            }
        }
        //
        // @audit CRITICAL Re-entrancy attack due to not following the Check-Effects-Interaction pattern
        //
        // ERC1155._mintBatch() will call _account.onERC1155BatchReceived() if
        // _account is contract. AttackContract.onERC1155BatchReceived() can hijack execution
        // flow by re-entering _splitValue() via HypercertMinter.splitValue() many times to mint a huge amount
        // of fractions for the same _tokenID as the decreased valueLeft has not been written to storage before
        // calling ERC1155._mintBatch()
        //

        _mintBatch(_account, toIDs, amounts, "");

        tokenValues[_tokenID] = valueLeft;

        emit BatchValueTransfer(typeIDs, fromIDs, toIDs, values);
    }

To prevent this valueLeft should be written to storage before calling ERC1155._mintBatch(). More examples: [1, 2, 3]