The owner of deposit in the BranchBridgeAgent.sol can be different than the value of _refundee address #364
Labels
bug
Something isn't working
downgraded by judge
Judge downgraded the risk level of this issue
duplicate-858
edited-by-warden
grade-a
Q-74
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/f5ba4de628836b2a29f9b5fff59499690008c463/src/BranchBridgeAgent.sol#L832
https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/BranchBridgeAgent.sol#L354
Vulnerability details
Impact
The transaction to the
retryDeposit
function can get reverted.Proof of Concept
When the user call
callOutAndBridge
function, user can choose value of_refundee
as the address to return excess gas deposited inmsg.value
to.https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/BranchBridgeAgent.sol#L210
The
callOutAndBridge
will call_createDeposit
to move assets from branch chain to root and create deposit data for user. in the deposit data, contract is using_refundee
value as owner of deposit transaction / data.https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/BranchBridgeAgent.sol#L832
Now if the user needs to retry the deposit transaction
The deposit that is already created
, the user should callretryDeposit
function. in this function, first, contract check if deposit belongs to message sender. ifMSG.SENDER
is the owner of the deposit, it's ok, else the transaction will get reverted.https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/BranchBridgeAgent.sol#L354
Now in the
callOutAndBridge
, if the user uses another address as_refundee
value, the owner of the deposit transaction will be another address, and the user is not able to useretryDeposit
function for the deposit transaction in future.Tools Used
Manually
Recommended Mitigation Steps
I think best option is to use
msg.sender
as the owner of deposit.https://github.com/code-423n4/2023-05-maia/blob/1a95ffeaa057f14e6f317f7c3af84def2db16309/src/ulysses-omnichain/BranchBridgeAgent.sol#L264
Assessed type
Other
The text was updated successfully, but these errors were encountered: