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

Insurance.getVaultDeltaForDeposit returns wrong investDelta #98

Open
code423n4 opened this issue Jul 7, 2021 · 3 comments
Open

Insurance.getVaultDeltaForDeposit returns wrong investDelta #98

code423n4 opened this issue Jul 7, 2021 · 3 comments
Labels
1 (Low Risk) bug Something isn't working duplicate This issue or pull request already exists

Comments

@code423n4
Copy link
Contributor

Handle

cmichel

Vulnerability details

Vulnerability Details

On single deposits, the getVaultDeltaForDeposit function is supposed to return 100% allocation for the desired vault to invest in.
However, it always sets 100% allocation to the first vault (index 0) because of using the wrong variable:

uint256[N_COINS] memory vaultIndexes;
// ...
// uses uninitialized vaultIndexes!
// same as investDelta[0] = 10000;
investDelta[vaultIndexes[0]] = 10000;

Impact

The investDelta also indicates that one would invest 100% in the first vault even if vaultIndexes suggests a different priority.

Recommended Mitigation Steps

Use _vaultIndexes (return value of sortVaultsByDelta) instead of the uninitialized vaultIndexes.

@kitty-the-kat
Copy link
Collaborator

kitty-the-kat commented Jul 14, 2021

investDelta isnt used, variable has been removed to not cause confusion

@kitty-the-kat kitty-the-kat added duplicate This issue or pull request already exists and removed disagree with severity sponsor disputed labels Jul 14, 2021
@flabble-gro
Copy link
Collaborator

Duplicate of #65

@flabble-gro flabble-gro marked this as a duplicate of #65 Jul 14, 2021
@ghoul-sol ghoul-sol reopened this Jul 25, 2021
@ghoul-sol
Copy link
Collaborator

Indeed, investDelta is returned but not used. Duplicate of #65 so low risk.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 (Low Risk) bug Something isn't working duplicate This issue or pull request already exists
Projects
None yet
Development

No branches or pull requests

4 participants