Fixed Gas amount implementation likely to lead operational failures

[0xM3R]

Vulnerability Details

The transferCrossChain function includes multiple external calls and relies on a fixed gas limit through gasLimitAmount. This design can lead to operational failures during high network demand and introduces risks due to inadequate external call validation.

Analysis

The vulnerable implementation is as follows:

standard-contracts/contracts/nft/contracts/evm/UniversalNFT.sol

Line 23 in a08166a

uint256 public immutable gasLimitAmount;

function transferCrossChain(
    address destination,
    address receiver,
    uint256 amount
) public {
    if (receiver == address(0)) revert InvalidAddress();
    _burn(msg.sender, amount);

    (address gasZRC20, uint256 gasFee) = IZRC20(destination)
        .withdrawGasFeeWithGasLimit(gasLimitAmount);
    if (destination != gasZRC20) revert InvalidAddress();

    if (!IZRC20(destination).transferFrom(msg.sender, address(this), gasFee))
        revert TransferFailed();
    if (!IZRC20(destination).approve(address(gateway), gasFee)) {
        revert ApproveFailed();
    }
    bytes memory message = abi.encode(receiver, amount, 0, msg.sender);

    CallOptions memory callOptions = CallOptions(gasLimitAmount, false);

    RevertOptions memory revertOptions = RevertOptions(
        address(this),
        true,
        address(0),
        abi.encode(amount, msg.sender),
        gasLimitAmount
    );

• The gasLimitAmount is immutable, preventing adaptation to fluctuating gas requirements.
• External calls (IZRC20 and gateway.call) lack comprehensive validation, exposing the function to potential failures or misbehavior from external contracts.

How It Can Be Harmful

• Operational Failures: Fixed gas limits may lead to transaction failures during high network congestion, leaving tokens burned without completing the transfer.
• External Call Risks: Unchecked external calls can result in unexpected reverts or malicious behavior.

How to Mitigate the Issue

1.Make Gas Limit Adjustable: Replace the immutable gasLimitAmount with a state variable and provide a setter function for the owner:

uint256 public gasLimitAmount;

function setGasLimit(uint256 gas) external onlyOwner {
    require(gas > 0, "Invalid gas limit");
    gasLimitAmount = gas;
}

2. Validate External Calls: Use try/catch blocks to handle failures gracefully and emit events for better debugging:

try gateway.call(...) {
    // Success logic
} catch Error(string memory reason) {
    emit GatewayCallFailed(reason);
} catch {
    emit GatewayCallFailed("Unknown error");
}

3.Log Gas Fee Usage: Emit detailed events to trace gas fee calculations and usage.

References:

• SWC-128: DoS with Block Gas Limit
  Highlights vulnerabilities caused by fixed gas limits and their impact on operational functionality.
  SWC Registry - SWC-128

• OpenZeppelin Adjustable Parameters
  Guidance on implementing adjustable parameters to improve contract flexibility.
  OpenZeppelin Blog

• CVE-2018-14732
  Denial of Service (DoS) vulnerability caused by insufficient gas limit validation.
  CVE-2018-14732

• CVE-2020-26272
  Highlights issues related to external contract interactions that may result in unintended behaviors or failures.
  CVE-2020-26272

[0xM3R]

The same flawed logic has been implemented in the UniversalToken.sol as well.

standard-contracts/contracts/token/contracts/zetachain/UniversalToken.sol

Line 24 in a08166a

uint256 public immutable gasLimitAmount;