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

fix: Prevent arbitrary calls to leverage zappers #409

Merged
merged 2 commits into from
Sep 9, 2024

Conversation

bingen
Copy link
Collaborator

@bingen bingen commented Sep 4, 2024

Closes #407

@bingen bingen added the Recon label Sep 4, 2024
@bingen bingen self-assigned this Sep 4, 2024
Copy link
Collaborator

@danielattilasimon danielattilasimon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! 👍 👍

} else {
revert("LZ: Wrong Operation");
}

// This will be used by the callback below no
receiver = IFlashLoanReceiver(msg.sender);

vault.flashLoan(this, tokens, amounts, userData);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally, I would have put the reset of receiver here, after the return of vault.flashLoan(). IMO it's more readable when the assignments are kept in the same function (easy to pair them together).

Plus, the current implementation relies on Balancer not returning successfully without having called us back. Now, in this case, it's easy to see by reading the Balancer source code that that's impossible, but in general, I prefer to minimize relying on external "promises" as much as possible.

For better readability (address PR #409 comments).
@bingen bingen merged commit a349602 into main Sep 9, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Recon] The flashloan protection is insufficient - We can operate on Troves we don't own
2 participants