search

code-423n4 / 2024-06-thorchain-findings

6 stars 3 forks source link

`batchTransferOutV5` could emit multiple `TransferOut` events, but Bifrost Observation can handle only one per transaction. #26

Closed howlbot-integration[bot] closed 3 months ago

howlbot-integration[bot] commented 3 months ago

Lines of code

https://github.com/code-423n4/2024-06-thorchain/blob/e5ae503d0dc2394a82242be6860eb538345152a1/ethereum/contracts/THORChain_Router.sol#L247-L253 https://github.com/code-423n4/2024-06-thorchain/blob/e5ae503d0dc2394a82242be6860eb538345152a1/ethereum/contracts/THORChain_Router.sol#L209-L238 https://github.com/code-423n4/2024-06-thorchain/blob/e5ae503d0dc2394a82242be6860eb538345152a1/bifrost/pkg/chainclients/shared/evm/smartcontract_log_parser.go#L166-L343

Vulnerability details

Impact

The batchTransferOutV5 function is the batch version of the transferOutV5 function. When provided with an array of TransferOutData, the batchTransferOutV5 function can emit multiple TransferOut events in one transaction.

However, in the GetTxInItem function in smartcontract_log_parser.go, if there are multiple transferOutEvents in the logs array, the information in txInItem is overwritten during the iteration of the last transferOutEvent. Consequently, only the last transferOutEvent is logged by Bifrost (its txInItem populated by GetTxInItem), and all previous ones are ignored.

As a result, when a THORChain vault uses batchTransferOutV5, their allowance could be spent without the THORChain network acknowledging the outbound from the vault, leading to a loss of funds for the THORChain vault.

Tools Used

Manual Review

Recommended Mitigation Steps

Modify the event logging logic in smartcontract_log_parser.go to accommodate the use of batchTransferOutV5.

Assessed type

Error

the-eridanus commented 3 months ago

This is a valid point, but it was disclosed that Bifrost had not yet been updated to support the batch functions so the bifrost portion of that functionality was not in scope, so I am not sure that this is a valid submission

the-eridanus commented 3 months ago

This is acknowledged as it will be resolved, but I'm not sure this warrants a pay out as it was called out that Bifrost had not yet been updated for Router V5 (so this was a known issue).

c4-judge commented 3 months ago

trust1995 marked the issue as satisfactory

c4-judge commented 3 months ago

trust1995 marked issue #8 as primary and marked this issue as a duplicate of 8

trust1995 commented 3 months ago

It's observed that the BiFrost integration with V5 was marked as out of scope in the Discord channel. Will invalidate accordingly.

c4-judge commented 3 months ago

trust1995 marked the issue as unsatisfactory: Out of scope