When calling the batchTransferOutV5 functions, some events may be filtered out

[c4-bot-10]

Lines of code

https://github.com/code-423n4/2024-06-thorchain/blob/e3fd3c75ff994dce50d6eb66eb290d467bd494f5/chain/ethereum/contracts/THORChain_Router.sol#L247 https://github.com/code-423n4/2024-06-thorchain/blob/e3fd3c75ff994dce50d6eb66eb290d467bd494f5/bifrost/pkg/chainclients/shared/evm/smartcontract_log_parser.go#L213

Vulnerability details

Impact

The router contract provides batchTransferOutV5 functions for handling multiple operations in a single transaction. This will trigger multiple TransferOut events in one transaction. But when using the GetTxInItem function to filter events, it is explicitly stated that it is not legal to have multiple transferOut events, transferOut event should be final. This will result in some correct operations being filtered out by the filter

Proof of Concept

Assuming the following situation：

1. Call batchTransferOutV5 to batch process three operations: assert=ETH amount = 1e18;assert = USDC amount = 1e16;assert = ETH amount = 2e18
2. Triggering three TransferOut events in a transaction after successful invocation
3. Smartcontractlog_parser Contract parsing transaction log

4. After reading the first transferOut event, earlyExit will be set to true and the loop will be terminated github:https://github.com/code-423n4/2024-06-thorchain/blob/e3fd3c75ff994dce50d6eb66eb290d467bd494f5/bifrost/pkg/chainclients/shared/evm/smartcontract_log_parser.go#L213

   
                    case transferOutEvent:
           // it is not legal to have multiple transferOut event , transferOut event should be final
           transferOutEvt, err := scp.parseTransferOut(*item)
           ...
           earlyExit = true
           isVaultTransfer = false
                       ...
                     }
                   if earlyExit {
           break
           }

5. Due to loop termination, the two transferOut events after batch operation are ignored

Tools Used

Manual audit

Recommended Mitigation Steps

Delete the batch processing function or cancel the logic of calling the end of the transferOut event when reading it

Assessed type

Other

[c4-judge]

trust1995 marked the issue as satisfactory

[c4-judge]

trust1995 marked the issue as selected for report

[c4-judge]

trust1995 marked the issue as unsatisfactory: Out of scope