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

setUnderlyingTokenPercent should check percentages #11

Open
code423n4 opened this issue Jul 5, 2021 · 1 comment
Open

setUnderlyingTokenPercent should check percentages #11

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

Comments

@code423n4
Copy link
Contributor

Handle

gpersoon

Vulnerability details

Impact

The function setUnderlyingTokenPercent doesn't check that the sum of all the percentages is 100%.
This way the percentages could be accidentally set up the wrong way, with unpredictable results.

Note: the function can only be called by controller or the owner so the likely hood of mistakes is pretty low.

Proof of Concept

//https://github.com/code-423n4/2021-06-gro/blob/main/contracts/insurance/Insurance.sol#L100
function setUnderlyingTokenPercent(uint256 coinIndex, uint256 percent) external override onlyValidIndex(coinIndex) {
require(msg.sender == controller || msg.sender == owner(), "setUnderlyingTokenPercent: !authorized");
underlyingTokensPercents[coinIndex] = percent;
emit LogNewTargetAllocation(coinIndex, percent);
}

Tools Used

Recommended Mitigation Steps

Change setUnderlyingTokenPercent to set the percentages for all the coins at the same time.
And check that the sum of the percentages is 100%

@flabble-gro
Copy link
Collaborator

Duplicate of #100

@flabble-gro flabble-gro marked this as a duplicate of #100 Jul 14, 2021
@ghoul-sol ghoul-sol reopened this Jul 25, 2021
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