Router insolvency due to not resetiing msgSender

[hats-bug-reporter[bot]]

Github username: -- Twitter username: -- Submission hash (on-chain): 0x17b9e36dece399af783816198c42277b1b255429dc40d097a86f64f1c7e167f8 Severity: high

Description: Description\ execute function has a variable called topLevel, it uses this variable to define flashloan calls and then uses it at the end of call to reset msgSender.

https://github.com/hats-finance/Spectra-0x4b792db3d2a5d1c1ccf9938380756b200c240e5d/blob/06b05fc261fe59b1fe25e308a10a16459289a4be/src/router/Router.sol#L169-L172

https://github.com/hats-finance/Spectra-0x4b792db3d2a5d1c1ccf9938380756b200c240e5d/blob/06b05fc261fe59b1fe25e308a10a16459289a4be/src/router/Router.sol#L187-L190

the issue here is that this variable is defined inside the function so if the function is called twice, the value will reset to default and at the end of the function, the msgSender will not reset.

Attack Scenario\

• user call execute

        bool topLevel;
        if (msgSender == address(0)) {
            msgSender = msg.sender;
            topLevel = true;

• it will call flashLoan and after that in onFlashLoan, execute will be called again. (topLevel will reset)

bool topLevel;

• so at the end msgSender will not reset.

Impact\

• Router insolvency

[yanisepfl]

Hello, We classified this issue as invalid. Indeed, by enforcing that only the contract itself can make nested calls, the contract prevents any external entity from re-entering execute maliciously:

else if (msg.sender != address(this)) {
    revert UnauthorizedReentrantCall();
}