cosmos / gosec

Apache License 2.0
19 stars 6 forks source link

Enhancement: check use of Quo functions occurring before Mul #59

Open johnsaigle opened 2 years ago

johnsaigle commented 2 years ago

It would be helpful to check for instances of precision loss when devs divide before using multiplication. Doing so in the context of cryptocurrencies can lead to loss of funds.

e.g. if we multiply before dividing, 12345 * 100 = 1234500. Then 1234500 / 100 gives 12345. however, if we divide before multiply, 12345 / 100, this becomes 123. Then if we multiply by 100, it becomes 12300. Precision loss occurs.

In Cosmos specifically there are a bunch of Quo* functions that do (quotient) division that will result in the above behavior.

A good practice for devs therefore is to do Mul operations before Quo operations to get the intuitive result that they are looking for. Doing it the other way should be an explicit choice and should be commented to show that a dev has considered the potential consequences.

odeke-em commented 2 years ago

Sounds good and thank you for filing this request @johnsaigle, I shall work on it.