code-423n4 / 2023-11-zetachain-findings

0 stars 0 forks source link

Security enhancements improve the robustness of the ZetaConnectorNonEth contract #265

Closed c4-bot-5 closed 11 months ago

c4-bot-5 commented 11 months ago

Lines of code

https://github.com/code-423n4/2023-11-zetachain/blob/main/repos/protocol-contracts/contracts/evm/ZetaConnector.non-eth.sol#L40 https://github.com/code-423n4/2023-11-zetachain/blob/main/repos/protocol-contracts/contracts/evm/ZetaConnector.non-eth.sol#L61

Vulnerability details

Impact

Consider these security enhancements to improve the robustness of the ZetaConnectorNonEth contract by guarding against common vulnerabilities and ensuring that the contract's functions are executed with valid and meaningful parameters.

  1. Validation in send Function Input Validation: Ensure input.zetaValueAndGas is greater than zero to avoid pointless transactions. Validate input.destinationChainId and input.destinationAddress for correctness.

  2. Validation in onReceive and onRevert Functions Supply Check: Before minting new tokens, verify that minting will not exceed the maxSupply. Input Validation: In onReceive, check that destinationAddress is not a zero address. In onRevert, validate remainingZetaValue is non-zero and zetaTxSenderAddress is not a zero address.

  3. Reentrancy Guard Implementation: Since the contract interacts with external token contracts (mint and burnFrom), consider using OpenZeppelin's ReentrancyGuard to prevent reentrancy attacks.

Proof of Concept

To implement security checks in the ZetaConnectorNonEth contract, consider the following enhancements:

// SPDX-License-Identifier: MIT pragma solidity 0.8.7;

import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import "@openzeppelin/contracts/security/ReentrancyGuard.sol";

// ... [rest of the imports] ...

contract ZetaConnectorNonEth is ZetaConnectorBase, ReentrancyGuard { // ... [existing contract code] ...

function send(ZetaInterfaces.SendInput calldata input) external override whenNotPaused nonReentrant {
    require(input.zetaValueAndGas > 0, "ZetaConnectorNonEth: Zeta value and gas must be greater than zero");

    ZetaNonEthInterface(zetaToken).burnFrom(msg.sender, input.zetaValueAndGas);

    emit ZetaSent(
        tx.origin,
        msg.sender,
        input.destinationChainId,
        input.destinationAddress,
        input.zetaValueAndGas,
        input.destinationGasLimit,
        input.message,
        input.zetaParams
    );
}

function onReceive(
    bytes calldata zetaTxSenderAddress,
    uint256 sourceChainId,
    address destinationAddress,
    uint256 zetaValue,
    bytes calldata message,
    bytes32 internalSendHash
) external override onlyTssAddress nonReentrant {
    require(destinationAddress != address(0), "ZetaConnectorNonEth: Invalid destination address");
    require(zetaValue + ZetaNonEthInterface(zetaToken).totalSupply() <= maxSupply, "ZetaConnectorNonEth: Exceeds max supply");

    ZetaNonEthInterface(zetaToken).mint(destinationAddress, zetaValue, internalSendHash);

    if (message.length > 0) {
        ZetaReceiver(destinationAddress).onZetaMessage(
            ZetaInterfaces.ZetaMessage(zetaTxSenderAddress, sourceChainId, destinationAddress, zetaValue, message)
        );
    }

    emit ZetaReceived(zetaTxSenderAddress, sourceChainId, destinationAddress, zetaValue, message, internalSendHash);
}

// ... [rest of the contract code with similar checks in onRevert] ...

}

Reasoning Behind Changes Input Validation: Ensures that the contract interacts only with valid addresses and values, preventing wasteful or erroneous transactions.

Reentrancy Guard: Protects against reentrancy attacks, a crucial security measure for contracts interacting with external contracts.

Supply Check: Prevents the minting of tokens beyond the set maximum supply, maintaining the integrity of the token's economics.

Tools Used

VS Code

Recommended Mitigation Steps

In the ZetaConnectorNonEth contract, the following key changes were made to enhance its security and functionality:

  1. Input Validation in send Change: Added a requirement that input.zetaValueAndGas must be greater than zero. Reason: This ensures that the send function is not called with a zero value, which would be ineffective and could potentially lead to errors or misuse.

  2. Validation in onReceive and onRevert Change: Implemented checks to: Ensure destinationAddress is not a zero address in onReceive. Validate remainingZetaValue is non-zero in onRevert. Confirm that minting new tokens will not exceed the maxSupply. Reason: Checking the destinationAddress and remainingZetaValue prevents interactions with invalid addresses and ensures meaningful transaction values. The maxSupply check maintains the integrity of the token supply, preventing over-minting and potential inflationary issues.

  3. Reentrancy Guard Change: Applied OpenZeppelin's ReentrancyGuard to the send, onReceive, and onRevert functions. Reason: Protects against reentrancy attacks, a significant security concern, especially in functions that interact with external contracts (like ERC20 token operations).

  4. Access Control for setMaxSupply Change: Retained the onlyTssAddress modifier for the setMaxSupply function. Reason: Ensures that only an authorized entity (TSS address) can update the maxSupply, maintaining proper access control and preventing unauthorized modifications to critical contract parameters.

Conclusion These enhancements to the ZetaConnectorNonEth contract are aimed at ensuring secure and proper functioning. By adding input validations and reentrancy guards, the contract becomes more robust against common vulnerabilities and misuse scenarios. Such precautions are particularly important in contracts that handle cross-chain interactions and token operations, where security and precision are paramount.

Assessed type

Access Control

c4-pre-sort commented 11 months ago

DadeKuma marked the issue as insufficient quality report

c4-judge commented 11 months ago

0xean marked the issue as unsatisfactory: Insufficient quality