Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Users may not receive the tokens in destination chain because of different address encoding #249

Closed
code423n4 opened this issue Sep 7, 2023 · 3 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-406 satisfactory satisfies C4 submission criteria; eligible for awards sufficient quality report This report is of sufficient quality

Comments

@code423n4
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2023-09-ondo/blob/b88271d64112234b7a7273cd7f3cea73c350e6a7/contracts/bridge/SourceBridge.sol#L79
https://github.com/code-423n4/2023-09-ondo/blob/b88271d64112234b7a7273cd7f3cea73c350e6a7/contracts/bridge/DestinationBridge.sol#L349

Vulnerability details

Impact

The bridge contracts assume that the source and destination chains have the same address encoding, which may not be true and cause the users to lose their tokens in the destination chain.

Proof of Concept

In SourceBridge, the source sender is set to msg.sender and encoded into the payload.

    bytes memory payload = abi.encode(VERSION, msg.sender, amount, nonce++);

https://github.com/code-423n4/2023-09-ondo/blob/b88271d64112234b7a7273cd7f3cea73c350e6a7/contracts/bridge/SourceBridge.sol#L79

In destination chain, the DestinationBridge will mint the new tokens to the same address as the source sender.

    TOKEN.mint(txn.sender, txn.amount);

https://github.com/code-423n4/2023-09-ondo/blob/b88271d64112234b7a7273cd7f3cea73c350e6a7/contracts/bridge/DestinationBridge.sol#L349

This will work fine if the bridges are only used in the chains that are EVM compatible and have the same address encoding.

However, if the bridges are used in the chains that are EVM compatible but have different address encoding, the users will not receive the tokens in the destination chain because there may be no address that matches the source sender in the destination chain.

For example, TRON is EVM compatible but has different address encoding from Ethereum. If the bridges are used in Ethereum and TRON, the users will not receive the tokens in TRON.
Currently, TVM is compatible with EVM.

https://tronprotocol.github.io/documentation-en/contracts/tvm/#features-of-tvm:
https://stackoverflow.com/questions/74974850/creating-a-wallet-on-ether-creates-it-on-all-evn-supported-networks

Because the contracts are deployed to any EVM compatible chains, it is possible that the bridges are used in the chains that have different address encoding.

Tools Used

Manual

Recommended Mitigation Steps

Allow users to set receiver address instead of using msg.sender address from source chain as the receiver address.

Assessed type

en/de-code

@code423n4 code423n4 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Sep 7, 2023
code423n4 added a commit that referenced this issue Sep 7, 2023
@c4-pre-sort
Copy link

raymondfam marked the issue as duplicate of #406

@c4-pre-sort
Copy link

raymondfam marked the issue as sufficient quality report

@c4-pre-sort c4-pre-sort added the sufficient quality report This report is of sufficient quality label Sep 8, 2023
@c4-judge
Copy link
Contributor

kirk-baird marked the issue as satisfactory

@c4-judge c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label Sep 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-406 satisfactory satisfies C4 submission criteria; eligible for awards sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

3 participants