Deposit owner is incorrectly set to _address to pay refunds to_ when depositing via BranchBridgeAgent
instead of caller or this behavior is not specified
#300
Labels
bug
Something isn't working
downgraded by judge
Judge downgraded the risk level of this issue
duplicate-858
grade-a
QA (Quality Assurance)
Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax
sufficient quality report
This report is of sufficient quality
Lines of code
https://github.com/code-423n4/2023-09-maia/blob/main/src/interfaces/IBranchBridgeAgent.sol#L154-L168
https://github.com/code-423n4/2023-09-maia/blob/main/src/interfaces/IBranchBridgeAgent.sol#L170-L184
https://github.com/code-423n4/2023-09-maia/blob/main/src/interfaces/IBranchBridgeAgent.sol#L186-L196
https://github.com/code-423n4/2023-09-maia/blob/main/src/interfaces/IBranchBridgeAgent.sol#L198-L214
https://github.com/code-423n4/2023-09-maia/blob/main/src/interfaces/IBranchBridgeAgent.sol#L216-L233
Vulnerability details
Summary
When depositing assets (single, multiple or signed/unsigned) to the Ulysses Omnichain system via
BranchBridgeAgent
s, in all deposit related functions, user is instructed to provide an address where excess gas from msg.value will be deposited to. Example forIBranchBridgeAgent::callOutAndBridge
):and for all other functions:
But in all these implementation, the deposit owner is actually set to this provided refund address, not the message sender,
while tokens are taken from the message sender.
If this behavior is intentional it must be documented as such since users of the protocol may end up giving ownership to addresses they did not intend to thus losing their deposits.
Impact
Users may incorrectly give ownership of their deposits to unwanted addresses that may lead to user funds being lost.
Tools Used
Manual review
Note for judging
This issue was confirmed by sponsors as being a documentation flaw in private. For the
IRootBridgeAgent
, for example, the full role of the address parameter was mentioned. However, as it is, the system user-interaction should be considered flawed for the Branch Agent and fixed as such.Recommendations
Either document this behavior, similar to how in
IRootBridgeAgent
it was mentioned, or modify that the owner to be set to the message sender and also document this accordingly.Assessed type
Other
The text was updated successfully, but these errors were encountered: