Lines of code
<https://github.com/code-423n4/2023-01-canto-identity/tree/main/src/CidNFT.sol#L193&gt;
<https://github.com/code-423n4/2023-01-canto-identity/tree/main/src/SubprotocolRegistry.sol#L87&gt;

Vulnerability details

The following contract use solmate’s SafeTransferLib : solmate/utils/SafeTransferLib.sol

Solmate’s safeTransfer() / safeTransferFrom uses inline assembly call to transfer ether from contract to receiver. According to Solidity Docs the call may return true even if it was a failure due to non-existing contract. This may result in ether being lost as the transfer is now considered finalized.

In CidNFT and SubprotocolRegistry, safeTransferFrom is used, however it does not check the existence of the code at the token address. This is a known issue when using solmate’s libraries.
This is stated in the Solmate library: <https://github.com/transmissions11/solmate/blob/main/src/utils/SafeTransferLib.sol#L9&gt;

Proof Of Concept

Use of solmate’s libraries:

    function safeTransferFrom(
        ERC20 token,
        address from,
        address to,
        uint256 amount
    ) internal {
        bool success;

        /// @solidity memory-safe-assembly
        assembly {
            // Get a pointer to some free memory.
            let freeMemoryPointer := mload(0x40)

            // Write the abi-encoded calldata into memory, beginning with the function selector.
            mstore(freeMemoryPointer, 0x23b872dd00000000000000000000000000000000000000000000000000000000)
            mstore(add(freeMemoryPointer, 4), from) // Append the "from" argument.
            mstore(add(freeMemoryPointer, 36), to) // Append the "to" argument.
            mstore(add(freeMemoryPointer, 68), amount) // Append the "amount" argument.

            success := and(
                // Set success to whether the call reverted, if not we check it either
                // returned exactly 1 (can't just be non-zero data), or had no return data.
                or(and(eq(mload(0), 1), gt(returndatasize(), 31)), iszero(returndatasize())),
                // We use 100 because the length of our calldata totals up like so: 4 + 32 * 3.
                // We use 0 and 32 to copy up to 32 bytes of return data into the scratch space.
                // Counterintuitively, this call must be positioned second to the or() call in the
                // surrounding and() call or else returndatasize() will be zero during the computation.
                call(gas(), token, 0, freeMemoryPointer, 100, 0, 32)
            )
        }

        require(success, "TRANSFER_FROM_FAILED");
    }

The following contracts use solmate’s transfer calls:

192: SafeTransferLib.safeTransferFrom(note, msg.sender, cidFeeWallet, cidFee);
193: SafeTransferLib.safeTransferFrom(note, msg.sender, subprotocolOwner, subprotocolFee - cidFee);

<https://github.com/code-423n4/2023-01-canto-identity/tree/main/src/CidNFT.sol#L192&gt;

<https://github.com/code-423n4/2023-01-canto-identity/tree/main/src/CidNFT.sol#L193&gt;

87: SafeTransferLib.safeTransferFrom(note, msg.sender, cidFeeWallet, REGISTER_FEE);

<https://github.com/code-423n4/2023-01-canto-identity/tree/main/src/SubprotocolRegistry.sol#L87&gt;

Recommended Mitigation Steps

Check for contract existence by adding and checking the codehash:

codehash := extcodehash(to)  

---

The text was updated successfully, but these errors were encountered:

All reactions