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.

Sign up for GitHub

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

Jump to bottom

Divide before multiply #74

Open
2 of 4 tasks
0xSandyy opened this issue Jun 5, 2024 · 2 comments · May be fixed by #77 or #86
Open
2 of 4 tasks

Divide before multiply #74

0xSandyy opened this issue Jun 5, 2024 · 2 comments · May be fixed by #77 or #86

Comments

@0xSandyy
Copy link
Contributor

0xSandyy commented Jun 5, 2024 •
edited
Loading

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
Copy link
Owner

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
Copy link
Contributor Author

0xSandyy commented Jun 6, 2024

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?

This was referenced Jun 6, 2024
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@kadenzipfel @0xSandyy