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

Flash loans can be used to make deposits with 0 timelock #180

Closed
code423n4 opened this issue Jan 6, 2022 · 2 comments
Closed

Flash loans can be used to make deposits with 0 timelock #180

code423n4 opened this issue Jan 6, 2022 · 2 comments
Labels
1 (Low Risk) Assets are not at risk. State handling, function incorrect as to spec, issues with comments bug Something isn't working duplicate This issue or pull request already exists sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")

Comments

@code423n4
Copy link
Contributor

Handle

pedroais

Vulnerability details

Impact

Anyone could use a flash loan to lock and unlock in the same block with a large amount and get an NFT with high points with funds that do not belong to him. Even if the multiplier for 0 timelock is 0 this could be done to get the NFT.

Proof of Concept

Point calculation formula :
return amount_ * (duration_ + _zeroDurationPointBase);

Recommended Mitigation Steps

Add a minimal timelock of 1 block to protect the contract from flash loans

@code423n4 code423n4 added 1 (Low Risk) Assets are not at risk. State handling, function incorrect as to spec, issues with comments bug Something isn't working labels Jan 6, 2022
code423n4 added a commit that referenced this issue Jan 6, 2022
@deluca-mike deluca-mike added invalid This doesn't seem right sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") and removed invalid This doesn't seem right labels Jan 8, 2022
@deluca-mike
Copy link
Collaborator

deluca-mike commented Jan 8, 2022

You know, I was about to write this: "This is expected, but also irrelevant, because we won't allow 0 duration, and even if we did, the gas costs to lock and unlock in the same block in a flash loan is probably way too high to justify the "loyalty" points one gets."

However, I thought about it some more and I have to agree that if we did allow a 0 duration, this entire score system is unenforceable due to flash loans. So, because of that, I'm going to make the score function simply amount_ * duration_, and remove _zeroDurationPointBase from the contract. If we wanted "mimial" lock duration, we can simply do something small like 1 day, or even 1 second. And if we did allow 0 seconds, then its only fair that "flash loaner" gets an NFT of 0 score.

@deluca-mike deluca-mike added the duplicate This issue or pull request already exists label Jan 9, 2022
@deluca-mike
Copy link
Collaborator

Duplicate #139

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 (Low Risk) Assets are not at risk. State handling, function incorrect as to spec, issues with comments bug Something isn't working duplicate This issue or pull request already exists sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")
Projects
None yet
Development

No branches or pull requests

3 participants