Issue with msg.value Reset in Dispatcher Contract During Flash Loan Execution

[hats-bug-reporter]

Github username: @0xmahdirostami
Twitter username: 0xmahdirostami
Submission hash (on-chain): 0x4a7a612fd62ef9364c23b61b1f39f0277f2ed86f38f288f403cc2da0f79386c3
Severity: high

Description:

Description

In the current implementation of the dispatcher contract, msg.value is used as a parameter for swaps on KyberSwap. However, if a user first initiates a flash loan, performs actions before repaying it, and assigns msg.value for a KyberSwap transaction, the expected msg.value is reset to zero. This is because msg.value does not persist across different transactions in a flash loan sequence and does not retain its initial value throughout the contract's execution.

Scenario

1. The victim user initiates a flash loan and provides various commands to execute before the loan repayment, assigning msg.value for later use in a KyberSwap swap.
2. After receiving the flash loan, msg.value is reset to zero (this can be seen in how the contract handles msg.sender within flash loans).

Spectra-0x4b792db3d2a5d1c1ccf9938380756b200c240e5d/src/router/Router.sol

Line 167 in 06b05fc

// Relying on msg.sender is problematic as it changes during a flash loan.

3. The contract attempts to perform the swap, but the msg.value used differs from the expected amountIn, which can prevent the swap from executing as intended.

Impact

If there is a KyberSwap action before the flash loan repayment, the dispatcher contract may be unable to execute the swap due to the discrepancy between msg.value and amountIn, as msg.value is reset to zero after the flash loan call. This could disrupt transaction flows for users and prevent successful completion of their commands.

Mitigation

To mitigate this, consider preserving the initial msg.value when the first transaction is initiated. You could save the initial msg.value at the start of the flash loan execution and use this stored value for subsequent commands, ensuring it remains unchanged across the execution sequence.

just like how the contract handles msg.sender

[yanisepfl]

Hello,

We classified this issue as Invalid because it does not result in theft of user funds.

For your information the flashloan feature we put at disposition of our users is meant for pool arbitrages, YT trading and PT interactions that never involve native ETH.

Thanks