#36501 [SC-Medium] Signature Front-Running Vulnerability in CollateralVault

Submitted on Nov 4th 2024 at 13:24:08 UTC by @Hoverfly9132 for Audit Comp | Anvil

• Report ID: #36501

• Report Type: Smart Contract

• Report severity: Medium

• Target: https://etherscan.io/address/0x5d2725fdE4d7Aa3388DA4519ac0449Cc031d675f

• Impacts:

  • Griefing (e.g. no profit motive for an attacker, but damage to the users or the protocol)

Description

Bug Description

The similar issue has found by the previous OpenZeppelin audit but they don't specify all the affected actions, and anvil team only fix the `depositAndStake` function, the other affected actions aren't reported and fixed in the previous audits, so this issue shouldn't be considered as known issue from my perspective.

The `modifyCollateralizableTokenAllowanceWithSignature` function in CollateralVault is vulnerable to signature front-running attacks. When the `TimeBasedCollateralPool` contract `stake` and `stakeReleasableTokensFrom` functions call the `modifyCollateralizableTokenAllowanceWithSignature` function to modify the allowance by signature, an attacker can monitor the mempool and front-run the transaction by calling the `modifyCollateralizableTokenAllowanceWithSignature` function first, then the original transaction will fail because the nonce is already used.

The vulnerability exists because:

1. The signature verification uses a nonce system that increments upon each use.

2. The `modifyCollateralizableTokenAllowanceWithSignature` function can be called by anyone to cause the signature nonce to be incremented.

3. Once a signature is used and the nonce is incremented, the original transaction will fail

Impact

Any users `stake` or `stakeReleasableTokensFrom` txs can be front-run by attackers to revert.

Recommendation

Fix as the `depositAndStake` function.

Proof of Concept

Proof of Concept

```solidity // SPDX-License-Identifier: MIT pragma solidity ^0.8.19;

import "forge-std/Test.sol";

abstract contract EIP712 { bytes32 private immutable _CACHED_DOMAIN_SEPARATOR; uint256 private immutable _CACHED_CHAIN_ID;

bytes32 private immutable _HASHED_NAME;
bytes32 private immutable _HASHED_VERSION;
bytes32 private immutable _TYPE_HASH;

constructor(string memory name, string memory version) {
    bytes32 hashedName = keccak256(bytes(name));
    bytes32 hashedVersion = keccak256(bytes(version));
    bytes32 typeHash = keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)");

    _HASHED_NAME = hashedName;
    _HASHED_VERSION = hashedVersion;
    _TYPE_HASH = typeHash;

    _CACHED_CHAIN_ID = block.chainid;
    _CACHED_DOMAIN_SEPARATOR = _buildDomainSeparator(typeHash, hashedName, hashedVersion);
}

function _domainSeparatorV4() internal view returns (bytes32) {
    if (block.chainid == _CACHED_CHAIN_ID) {
        return _CACHED_DOMAIN_SEPARATOR;
    } else {
        return _buildDomainSeparator(_TYPE_HASH, _HASHED_NAME, _HASHED_VERSION);
    }
}

function _buildDomainSeparator(
    bytes32 typeHash,
    bytes32 nameHash,
    bytes32 versionHash
) private view returns (bytes32) {
    return keccak256(
        abi.encode(
            typeHash,
            nameHash,
            versionHash,
            block.chainid,
            address(this)
        )
    );
}

function _hashTypedDataV4(bytes32 structHash) internal view virtual returns (bytes32) {
    return keccak256(abi.encodePacked("\x19\x01", _domainSeparatorV4(), structHash));
}

}

abstract contract SignatureNonces { mapping(address => mapping(bytes32 => uint256)) private _accountTypeNonces;

function nonces(address owner, bytes32 signatureType) public view virtual returns (uint256) {
    return _accountTypeNonces[owner][signatureType];
}

function _useNonce(address owner, bytes32 signatureType) internal virtual returns (uint256) {
    return _accountTypeNonces[owner][signatureType]++;
}

}

contract CollateralVault is EIP712, SignatureNonces { bytes32 public constant COLLATERALIZABLE_TOKEN_ALLOWANCE_ADJUSTMENT_TYPEHASH = keccak256( "CollateralizableTokenAllowanceAdjustment(address collateralizableAddress,address tokenAddress,int256 allowanceAdjustment,uint256 approverNonce)" );

mapping(address => mapping(address => mapping(address => uint256))) public accountCollateralizableTokenAllowances;
mapping(address => bool) public collateralizableContracts;

error InvalidSignature(address signer);
error ContractNotApprovedByProtocol(address contractAddress);

constructor() EIP712("CollateralVault", "1") {}

function modifyCollateralizableTokenAllowanceWithSignature(
    address _accountAddress,
    address _collateralizableContractAddress,
    address _tokenAddress,
    int256 _allowanceAdjustment,
    bytes calldata _signature
) external {
    if (_allowanceAdjustment > 0 && !collateralizableContracts[_collateralizableContractAddress])
        revert ContractNotApprovedByProtocol(_collateralizableContractAddress);

    _modifyCollateralizableTokenAllowanceWithSignature(
        _accountAddress,
        _collateralizableContractAddress,
        _tokenAddress,
        _allowanceAdjustment,
        _signature
    );
}

function _modifyCollateralizableTokenAllowanceWithSignature(
    address _accountAddress,
    address _collateralizableContractAddress,
    address _tokenAddress,
    int256 _allowanceAdjustment,
    bytes calldata _signature
) private {
    console.log("nonce: ", nonces(_accountAddress, COLLATERALIZABLE_TOKEN_ALLOWANCE_ADJUSTMENT_TYPEHASH));
    bytes32 hash = _hashTypedDataV4(
        keccak256(
            abi.encode(
                COLLATERALIZABLE_TOKEN_ALLOWANCE_ADJUSTMENT_TYPEHASH,
                _collateralizableContractAddress,
                _tokenAddress,
                _allowanceAdjustment,
                _useNonce(_accountAddress, COLLATERALIZABLE_TOKEN_ALLOWANCE_ADJUSTMENT_TYPEHASH)
            )
        )
    );

    address signer = _recoverSigner(hash, _signature);
    if (signer != _accountAddress) {