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

6 stars 3 forks source link

The `TransferOutAndCallV5` event is not caught by `smartcontract_log_parser.go` #20

Closed howlbot-integration[bot] closed 4 months ago

howlbot-integration[bot] commented 4 months ago

Lines of code

https://github.com/code-423n4/2024-06-thorchain/blob/main/bifrost/pkg/chainclients/shared/evm/smartcontract_log_parser.go#L22-L26 https://github.com/code-423n4/2024-06-thorchain/blob/main/bifrost/pkg/chainclients/shared/evm/smartcontract_log_parser.go#L295-L337

Vulnerability details

Impact

The contract and more precisely the events of the version 5 of the protocol are not integrated as expected.

Actions that should be undertaken by the Bifrost are ignored, leading to inconsistencies in the entire protocol such as accounting errors and potential loss of funds.

Proof of concept

In the previous version of the protocol, the smartcontract_log_parser.go was responsible for listening to multiple events emitted by THORChain_Router.sol :

https://github.com/code-423n4/2024-06-thorchain/blob/main/bifrost/pkg/chainclients/shared/evm/smartcontract_log_parser.go#L22-L26

const (
    NativeTokenAddr         = "0x0000000000000000000000000000000000000000"
    depositEvent            = "0xef519b7eb82aaf6ac376a6df2d793843ebfd593de5f1a0601d3cc6ab49ebb395" // cast keccak "Deposit(address,address,uint256,string)"
    transferOutEvent        = "0xa9cd03aa3c1b4515114539cd53d22085129d495cb9e9f9af77864526240f1bf7" // cast keccak "TransferOut(address,address,address,uint256,string)"
    transferAllowanceEvent  = "0x05b90458f953d3fcb2d7fb25616a2fddeca749d0c47cc5c9832d0266b5346eea" // cast keccak "TransferAllowance(address,address,address,uint256,string)"
    vaultTransferEvent      = "0x281daef48d91e5cd3d32db0784f6af69cd8d8d2e8c612a3568dca51ded51e08f" // cast keccak "VaultTransfer(address,address,(address,uint256)[],string)"
    transferOutAndCallEvent = "0x8e5841bcd195b858d53b38bcf91b38d47f3bc800469b6812d35451ab619c6f6c" // cast keccak "TransferOutAndCall(address,address,uint256,address,address,uint256,string)"
)

Once one of these events has been intercepted, it is parsed and actions are undertaken by Bifrost depending on the nature of the emission.

One of these actions is transferOutAndCallEvent, described here and is triggered in the transferOutAndCall() function of the smart contract.

The version 5 of the protocol introduced a similar event in a new function of the smart contract called _transferOutAndCallV5() which emits TransferOutAndCallV5.

As you can see, this particular event does not figure in the list of events in smartcontract_log_parser.go and is thus never intercepted nor processed while it should be.

Tools used

Manual review

Recommended mitigation steps

Define the corresponding event in smartcontract_log_parser.go and implement the actions to undertake when it is intercepted.

Assessed type

Context

the-eridanus commented 4 months ago

While this will be fixed, it was called out that Bifrost had not yet been updated for Router V5, so this was a known issue already

trust1995 commented 4 months ago

@the-eridanus could you explain the rationale for considering this a known issue? Without it, there are grounds to keep as valid.

c4-judge commented 4 months ago

trust1995 marked the issue as satisfactory

c4-judge commented 4 months ago

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

trust1995 commented 4 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 4 months ago

trust1995 marked the issue as unsatisfactory: Out of scope