Lodestar-Finance / lodestar-protocol

Houses the code for the Lodestar Finance DeFi protocol.
BSD 3-Clause "New" or "Revised" License
10 stars 7 forks source link

Division before Multiplication can lead to 0 return price and a not-accurate price #19

Closed maarcweiss closed 1 year ago

maarcweiss commented 1 year ago

TITLE (Division before Multiplication can lead to 0 return price and a not-accurate price)

The vulnerability relies on the following contract: https://github.com/LodestarFinance/plvglp-oracle/blob/b9e78d6bfb119ee1703a7e97d283a1e1dcf25046/contracts/plvGLPOracle.sol#L65

The problem is that when trying to get the price returned there is a division before multiplication. Which itself alone would be a medium-severity issue. Example of a similar issue: https://solodit.xyz/issues/6304

The problem is that it might return 0 or a very imprecise result in an oracle function that returns price. Therefore, funds will be compromised due to the inaccuracy of pricing in some cases. Very similar issue to the following: https://solodit.xyz/issues/2835 The error in pricing also comes from the fact that solidity rounds down. To read: https://medium.com/@soliditydeveloper.com/solidity-design-patterns-multiply-before-dividing-407980646f7

uint256 price = (glpAUM / glpSupply) * DECIMAL_DIFFERENCE;

SEVERITY (either high or medium, see the rules)

HIGH, loss of funds due to oracle imprecision via arithmetic errors

A LINK TO THE GITHUB ISSUE

https://github.com/LodestarFinance/plvglp-oracle/blob/b9e78d6bfb119ee1703a7e97d283a1e1dcf25046/contracts/plvGLPOracle.sol#L65

SOLUTION

Perform the multiplication before the division

uint256 price = (glpAUM * DECIMAL_DIFFERENCE) / glpSupply;

LodestarFinance commented 1 year ago

This issue pertains to a different repository, and has already been noted.

maarcweiss commented 1 year ago

I am sorry, it is in scope:

image

Issue is in scope because you had it on the contracts in hats UI. You can't say no now. This is not how things work. Kindly recover the issue, because it is existent and is scope.

I guess you meant that I have to report the issue to another repo, not to this one. Is that right?

MagicQuanta commented 1 year ago

@maarcweiss correct, someone else already reported it in the correct repo