Skip to content

Conversation

R1sco
Copy link

@R1sco R1sco commented Oct 18, 2025

Summary

Add validation checks to prevent transfers to zero address and invalid chain IDs in Token Bridge functions.

Problem

Currently, the Bridge contract accepts bytes32(0) as recipient address and 0 as chain ID without validation. This allows users to accidentally send funds to unrecoverable destinations through:

  • Programming errors (uninitialized variables)
  • Copy-paste mistakes
  • Direct contract interactions via Etherscan

Solution

Implement defense-in-depth by adding input validation at the smart contract level:

  • require(recipient != bytes32(0), "invalid recipient")
  • require(recipientChain != 0, "invalid chain")

Changes

  • transferTokens() - add recipient and chain validation
  • wrapAndTransferETH() - add recipient and chain validation
  • transferTokensWithPayload() - add recipient and chain validation
  • wrapAndTransferETHWithPayload() - add recipient and chain validation

Testing

All existing tests pass:

forge test --match-path "forge-test/Bridge.t.sol"

Impact

  • Prevents permanent fund loss from user error
  • Minimal gas overhead (~200-300 gas per transfer)
  • No breaking changes to existing functionality
  • Aligns with industry standards (OpenZeppelin, Uniswap)

Related

This fix addresses a lack of defense-in-depth issue identified through security research.

…er functions

Add validation checks to prevent transfers to zero address and invalid chain IDs.
This implements defense-in-depth by validating user inputs at the contract level.

Changes:
- Add require(recipient != bytes32(0)) to all transfer functions
- Add require(recipientChain != 0) to all transfer functions
- Applies to: transferTokens, wrapAndTransferETH, transferTokensWithPayload, wrapAndTransferETHWithPayload

Prevents permanent fund loss from user error when accidentally providing zero address or invalid chain ID.
Copy link
Contributor

@evan-gray evan-gray left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other contributors: I would recommend against these changes for the reasons listed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is there a new gitmodules here?

uint256 arbiterFee,
uint32 nonce
) public payable returns (uint64 sequence) {
require(recipient != bytes32(0), "invalid recipient");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not prohibited in the spec for an EVM chain or another platform’s token bridge implementation to use the zero address as a valid recipient

uint32 nonce
) public payable returns (uint64 sequence) {
require(recipient != bytes32(0), "invalid recipient");
require(recipientChain != 0, "invalid chain");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, there may be some reason to allow for a chain id 0 transfer and it is not prohibited in the spec

@R1sco R1sco closed this Oct 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants