Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Router insolvency due to not resetiing msgSender #3

Open
hats-bug-reporter bot opened this issue Nov 11, 2024 · 1 comment
Open

Router insolvency due to not resetiing msgSender #3

hats-bug-reporter bot opened this issue Nov 11, 2024 · 1 comment
Labels
bug Something isn't working invalid This doesn't seem right

Comments

@hats-bug-reporter
Copy link

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.

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

if (topLevel) {
// top-level reset
msgSender = address(0);
}

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
@hats-bug-reporter hats-bug-reporter bot added the bug Something isn't working label Nov 11, 2024
@yanisepfl yanisepfl added the invalid This doesn't seem right label Nov 13, 2024
@yanisepfl
Copy link
Collaborator

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();
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

1 participant