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

Token recipient is an inaccessible address for contracts #514

Closed
c4-submissions opened this issue Sep 7, 2023 · 4 comments
Closed

Token recipient is an inaccessible address for contracts #514

c4-submissions opened this issue Sep 7, 2023 · 4 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

@c4-submissions
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2023-09-ondo/blob/47d34d6d4a5303af5f46e907ac2292e6a7745f6c/contracts/bridge/SourceBridge.sol#L61-L82
https://github.com/code-423n4/2023-09-ondo/blob/47d34d6d4a5303af5f46e907ac2292e6a7745f6c/contracts/bridge/DestinationBridge.sol#L85-L114
https://github.com/code-423n4/2023-09-ondo/blob/47d34d6d4a5303af5f46e907ac2292e6a7745f6c/contracts/bridge/DestinationBridge.sol#L337-L353

Vulnerability details

Impact

The msg.sender address from the SourceBridge.burnAndCallAxelar function is used by the DestinationBridge._mintIfThresholdMet function as the TOKEN recipient. However, the msg.sender address will not be controllable by contracts on L2, so any tokens will be lost.

Proof of Concept

The SourceBridge.burnAndCallAxelar function constructs the payload:

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

The payload then is decoded at the DestinationBridge._execute function and used to save transaction parameters in the txnHashToTransaction mapping as the Transaction(srcSender, amt) structure.

90    (bytes32 version, address srcSender, uint256 amt, uint256 nonce) = abi
91      .decode(payload, (bytes32, address, uint256, uint256));

109    txnHashToTransaction[txnHash] = Transaction(srcSender, amt);

The DestinationBridge._mintIfThresholdMet uses the parameters for the tokens minting:

339    Transaction memory txn = txnHashToTransaction[txnHash];

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

So if the burnAndCallAxelar function is called by a contract the tokens will be minted to a wrong address (not the contract address at the destination chain ).

Tools Used

Manual review

Recommended Mitigation Steps

Consider specifying recipients for any token transfers or reverting in the case of the sender is not an EOA.

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
@c4-pre-sort
Copy link

raymondfam marked the issue as duplicate of #119

@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 duplicate of #406

@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