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

Multicall feature in SourceBridge may be abused to send arbitrary messages to the Axelar service #445

Open
c4-submissions opened this issue Sep 7, 2023 · 7 comments
Labels
bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue grade-a low quality report This report is of especially low quality primary issue Highest quality submission among a set of duplicates QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax

Comments

@c4-submissions
Copy link
Contributor

Lines of code

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

Vulnerability details

The SourceBridge contract includes a function to allow a "multicall" feature from the contract. Even though this function is access protected, it could be abused to send any arbitrary message to the Axelar service, enabling arbitrary minting through the bridge.

Impact

The SourceBridge contract is in charge of implementing token bridging in the source chain: it burns tokens from the caller and relays the message to the destination bridge in the destination chain using the Axelar service. The DestinationBridge contract validates the message is coming from the registered source bridge address for the given chain and proceeds with the minting in the destination chain.

The SourceBridge contract also contains a "multicall" feature to allow execution of multiple transactions in a batched fashion:

https://github.com/code-423n4/2023-09-ondo/blob/main/contracts/bridge/SourceBridge.sol#L160-L171

160:   function multiexcall(
161:     ExCallData[] calldata exCallData
162:   ) external payable override onlyOwner returns (bytes[] memory results) {
163:     results = new bytes[](exCallData.length);
164:     for (uint256 i = 0; i < exCallData.length; ++i) {
165:       (bool success, bytes memory ret) = address(exCallData[i].target).call{
166:         value: exCallData[i].value
167:       }(exCallData[i].data);
168:       require(success, "Call Failed");
169:       results[i] = ret;
170:     }
171:   }

This feature allows to make arbitrary calls to any recipient on behalf of the SourceBridge. When combined with the contract's capability to send messages through the bridge, it introduces the risk of relaying arbitrary messages through the bridge, potentially leading to arbitrary token minting in the destination chain.

Even though the multiexcall() function is access protected, this represents a huge risk since the contract interfaces directly with the bridge service and may lead to severe consequences if abused.

Proof of Concept

  1. A malicious owner or compromised account uses multiexcall() to send a call to AXELAR_GATEWAY.callContract() using the same payload that is constructed by using burnAndCallAxelar().
  2. Attacker mints the tokens in the destination chain without burning anything in the source chain.

Recommended Mitigation Steps

Remove the multiexcall() function from the SourceBridge contract.

Assessed type

Other

@c4-submissions c4-submissions 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
c4-submissions added a commit that referenced this issue Sep 7, 2023
@raymondfam
Copy link

From readme:

Centralization Risk - we are aware that our management functions and contract upgradeability results in a significantly more centralized system than Compound V2.

@c4-pre-sort
Copy link

raymondfam marked the issue as low quality report

@c4-pre-sort c4-pre-sort added low quality report This report is of especially low quality primary issue Highest quality submission among a set of duplicates labels Sep 8, 2023
@c4-pre-sort
Copy link

raymondfam marked the issue as primary issue

@c4-judge c4-judge added the unsatisfactory does not satisfy C4 submission criteria; not eligible for awards label Sep 19, 2023
@c4-judge
Copy link
Contributor

kirk-baird marked the issue as unsatisfactory:
Invalid

@c4-judge c4-judge reopened this Sep 27, 2023
@c4-judge c4-judge removed the unsatisfactory does not satisfy C4 submission criteria; not eligible for awards label Sep 27, 2023
@c4-judge
Copy link
Contributor

kirk-baird removed the grade

@c4-judge c4-judge added downgraded by judge Judge downgraded the risk level of this issue and removed 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value labels Sep 27, 2023
@c4-judge
Copy link
Contributor

kirk-baird changed the severity to QA (Quality Assurance)

@c4-judge c4-judge added the QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax label Sep 27, 2023
@c4-judge
Copy link
Contributor

kirk-baird marked the issue as grade-a

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue grade-a low quality report This report is of especially low quality primary issue Highest quality submission among a set of duplicates QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax
Projects
None yet
Development

No branches or pull requests

4 participants