Divide before multiply

[0xSandyy]

Checklist

• I have searched the existing issues and pull requests for duplicates.

Type of Issue

• New vulnerability addition
• Feature request
• Update existing vulnerability

Description

Division before multiplication

Solidity's integer division truncates. Thus, performing division before multiplication can lead to precision loss.

contract A {
	function func(uint n) public {
        coins = (oldSupply / n) * interest; // causes precision loss
    }
}

If n is greater than oldSupply, coins will be zero. Also, the fractional part is truncated due to integer division in solidity.

1 / 3 = 0 // rounding to 0
3 / 2 = 1 // 0.5 is truncated

Let's expand further,
When oldSupply = 5; n = 10, interest = 2,
if (oldSupply / n) * interest is used, coins value will round to zero.
and If (oldSupply * interest / n) is used, coins value will be 1.

Similarly for larger values,
When oldSupply = 119, n = 10, interest = 10,
if (oldSupply / n) * interest is used, coins value will be 110
and if (oldSupply * interest / n) is used, coins value will be 119.

Here, 9 coins were lost due to division before multiplication but following multiplication before division gave the exact value of coins. Thus, multiplication before division can prevent loss of precision due to truncation.

[kadenzipfel]

Nice, I think we can generalize this to "Integer Truncation" and include other ways for this to happen without necessarily being a multiplication/division ordering issue. Feel free to make a PR

[0xSandyy]

Hi, It's a good idea to generalize this to "Integer Truncation". For now, I am going to create a PR with the issue as it is. We can make the change to generalize this as "Integer Truncation" later after other ways to precision loss are added to this repository or we can change it in the PR. Let me know if the change is necessary.

As of right now, The only vulnerability I can think of that causes precision loss is some sort of rounding error. But, I think the description and concept will be somewhat similar to this. Should I create a new issue for rounding errors or leave this as it is? Also, can you give me something more over the top of your head that might cause solidity truncation errors?