Forwarder Verifying without Signed `from`

Smart Contracts Review Wanted
1

We have a very specific use case whereby we get both the address and the signature of a given meta transaction at the same time. Put another way, we don't know the address of the EOA until after the signature is complete.

This means that we do not know the from address in order to include it in the EIP-712 signature for a typical Forwarder, although we do know it at the point at which the transaction is submitted to the forwarder such that it can be appended as required per ERC-2771.

I wanted to explore any concerns regarding vulnerabilities that might emerge from a Forwarder implementation which omits from from within the signed digest as described. My sense is that this doesn't raise security concerns since we must still recover the signature against the from address submitted to the Forwarder and that submitting a bad from address will halt the verification.

I found it easier to modify the MinimalForwarder from 4 as an example:

// SPDX-License-Identifier: MIT
// OpenZeppelin Contracts (last updated v4.9.0) (metatx/MinimalForwarder.sol)

pragma solidity ^0.8.17;

import "../utils/cryptography/ECDSA.sol";
import "../utils/cryptography/EIP712.sol";

/**
 * @dev Simple minimal forwarder to be used together with an ERC2771 compatible contract. See {ERC2771Context}.
 *
 * MinimalForwarder is mainly meant for testing, as it is missing features to be a good production-ready forwarder. This
 * contract does not intend to have all the properties that are needed for a sound forwarding system. A fully
 * functioning forwarding system with good properties requires more complexity. We suggest you look at other projects
 * such as the GSN which do have the goal of building a system like that.
 */
contract MinimalForwarder is EIP712 {
    using ECDSA for bytes32;

    struct ForwardRequest {
        address from;
        address to;
        uint256 value;
        uint256 gas;
        uint256 nonce;
        bytes data;
    }

    // Included for demonstrative purposes, this is what we actually sign -- without `from`
    // struct ForwardRequestSig {
    //     address to;
    //     uint256 value;
    //     uint256 gas;
    //     uint256 nonce;
    //     bytes data;
    // }

    bytes32 private constant _TYPEHASH =
        keccak256("ForwardRequest(address to,uint256 value,uint256 gas,uint256 nonce,bytes data)");

    mapping(address => uint256) private _nonces;

    constructor() EIP712("MinimalForwarder", "0.0.1") {}

    function getNonce(address from) public view returns (uint256) {
        return _nonces[from];
    }

    function verify(ForwardRequest calldata req, bytes calldata signature) public view returns (bool) {
        address signer = _hashTypedDataV4(
            // `from` removed from the digest used in the signed message
            keccak256(abi.encode(_TYPEHASH, req.to, req.value, req.gas, req.nonce, keccak256(req.data)))
        ).recover(signature);
        return _nonces[req.from] == req.nonce && signer == req.from;
    }

    function execute(
        ForwardRequest calldata req,
        bytes calldata signature // See ForwardRequestSig for what we've actually signed 
    ) public payable returns (bool, bytes memory) {
        require(verify(req, signature), "MinimalForwarder: signature does not match request");
        _nonces[req.from] = req.nonce + 1;

        (bool success, bytes memory returndata) = req.to.call{gas: req.gas, value: req.value}(
            abi.encodePacked(req.data, req.from)
        );

        // Validate that the relayer has sent enough gas for the call.
        // See https://ronan.eth.limo/blog/ethereum-gas-dangers/
        if (gasleft() <= req.gas / 63) {
            // We explicitly trigger invalid opcode to consume all gas and bubble-up the effects, since
            // neither revert or assert consume all gas since Solidity 0.8.0
            // https://docs.soliditylang.org/en/v0.8.0/control-structures.html#panic-via-assert-and-error-via-require
            /// @solidity memory-safe-assembly
            assembly {
                invalid()
            }
        }

        return (success, returndata);
    }
}
2

I've come across one issue testing this here; a nonce in the signature is also no longer possible introducing replay concerns. I've seen other relayer services like Gelato use a salt instead which seems like a viable alternative.

3

For anyone curious, here is what I ended up with:

// SPDX-License-Identifier: MIT
// OpenZeppelin Contracts (last updated v4.9.0) (metatx/MinimalForwarder.sol)

pragma solidity ^0.8.17;

import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";
import "@openzeppelin/contracts/utils/cryptography/EIP712.sol";

/**
 * @dev Simple minimal forwarder to be used together with an ERC2771 compatible contract. See {ERC2771Context}.
 *
 * This MinimalForwarder has been aadapted from the OpenZeppelin example to be tailored for use with HaLo chips.
 * In a single tap with version c3 and greater HaLo chips you can obtain both the Ethereum address and a signature
 * from the chip, however, the Ethereum address is not signed and is passed independently as a parameter.
 * 
 * The `from` parameter is not signed with the rest of the data. A deadline has been added to mitigate sending stale
 * requests.
 */
contract MinimalForwarder is EIP712 {
    using ECDSA for bytes32;

    struct ECDSASignature {
        bytes32 r;
        bytes32 s;
        uint8 v;
    }

    // Note that `from` is not signed
    struct ForwardRequest {
        address from;
        address to;
        uint256 value;
        uint256 gas;
        uint48 deadline;
        uint256 salt;
        bytes data;
    }

    bytes32 private constant _TYPEHASH =
        keccak256("ForwardRequest(address to,uint256 value,uint256 gas,uint48 deadline,uint256 salt,bytes data)");

    mapping(address => mapping(uint256 => bool)) private _usedSalts;

    constructor() EIP712("MinimalForwarder", "0.0.2") {}

    function checkSalt(address from, uint256 salt) public view returns (bool) {
        return _usedSalts[from][salt];
    }

    function verify(ForwardRequest calldata req, ECDSASignature calldata signature) public view returns (bool) { 
        require(req.deadline >= block.timestamp, "MinimalForwarder: request expired");
        address signer = _hashTypedDataV4(
            keccak256(abi.encode(_TYPEHASH, req.to, req.value, req.gas, req.deadline, req.salt, keccak256(req.data)))
        ).recover(signature.v, signature.r, signature.s);
        
        // Check if the salt has been used before.
        bool saltUsed = _usedSalts[req.from][req.salt];
        return !saltUsed && signer == req.from;
    }

    function execute(
        ForwardRequest calldata req,
        ECDSASignature calldata signature // See ForwardRequestSig for what we've actually signed 
    ) public payable returns (bool, bytes memory) {
        require(verify(req, signature), "MinimalForwarder: signature does not match request");
        // Store the salt for this `from` + `salt` pair, to prevent replay attacks.
        _usedSalts[req.from][req.salt] = true;

        (bool success, bytes memory returndata) = req.to.call{gas: req.gas, value: req.value}(
            abi.encodePacked(req.data, req.from)
        );

        // Validate that the relayer has sent enough gas for the call.
        // See https://ronan.eth.limo/blog/ethereum-gas-dangers/
        if (gasleft() <= req.gas / 63) {
            // We explicitly trigger invalid opcode to consume all gas and bubble-up the effects, since
            // neither revert or assert consume all gas since Solidity 0.8.0
            // https://docs.soliditylang.org/en/v0.8.0/control-structures.html#panic-via-assert-and-error-via-require
            /// @solidity memory-safe-assembly
            assembly {
                invalid()
            }
        }

        return (success, returndata);
    }
}

I implemented a salt that gets stored with each signature to prevent replay attacks. I also added a deadline to mirror the v6 forwarder and preclude anyone from keeping signatures around too long.