Skip to content
This repository has been archived by the owner on May 5, 2024. It is now read-only.

sil3th - Precision Loss Due to Non-Exact Multiples in Division #17

Closed
sherlock-admin2 opened this issue Nov 4, 2023 · 1 comment
Closed
Labels
Excluded Excluded by the judge without consulting the protocol or the senior Non-Reward This issue will not receive a payout

Comments

@sherlock-admin2
Copy link
Contributor

sherlock-admin2 commented Nov 4, 2023

sil3th

medium

Precision Loss Due to Non-Exact Multiples in Division

Summary

When prizePool and currentRoundAgentsAlive are not exact multiples of each other, the division operation will lead to precision loss. Solidity performs integer division by truncating the fractional part, meaning it discards the remainder. This behavior can result in incorrect calculations when the intention is to maintain the fractional part.

Vulnerability Detail

Explained in the POC Below

`pragma solidity ^0.8.0;

contract PrecisionLossExample {
uint256 public prizePool;
uint256 public currentRoundAgentsAlive;
uint256 public totalEscapeValue;

constructor(uint256 _prizePool, uint256 _currentRoundAgentsAlive) {
prizePool = _prizePool;
currentRoundAgentsAlive = _currentRoundAgentsAlive;
totalEscapeValue = prizePool / currentRoundAgentsAlive;
}

function calculateTotalEscapeValue() public view returns (uint256) {
return totalEscapeValue;
}
}

`
In this example, the prizePool is set to 100, and currentRoundAgentsAlive is set to 30. When we deploy the contract and calculate totalEscapeValue, we can observe that precision loss occurs due to integer division:

`// Deployment
PrecisionLossExample example = new PrecisionLossExample(100, 30);

// Retrieve the calculated totalEscapeValue
uint256 result = example.calculateTotalEscapeValue(); // result will be 3
`
In this case, the result of the division is 3, which is the integer part of the result. The fractional part is discarded due to integer division, resulting in precision loss.

Impact

Precision loss can lead to inaccurate calculation of totalEscapeValue.

Code Snippet

https://github.com/sherlock-audit/2023-10-looksrare/blob/86e8a3a6d7880af0dc2ca03bf3eb31bc0a10a552/contracts-infiltration/contracts/Infiltration.sol#L740C31-L740C31

Tool used

Manual Review

Recommendation

Scale Up Values Before Division: Multiply both prizePool and currentRoundAgentsAlive by a common scaling factor before performing the division. This scaling factor should be a power of 10 to ensure precision is maintained.

@github-actions github-actions bot closed this as completed Nov 6, 2023
@github-actions github-actions bot added the Excluded Excluded by the judge without consulting the protocol or the senior label Nov 6, 2023
@nevillehuang
Copy link
Collaborator

nevillehuang commented Nov 6, 2023

#17 - An example is given, but impact is not significant enough. Precision used to indicate finding is also incorrect since example does not correctly reflect actual possible precision loss of prizePool (which is 18 decimals since it is native ETH/wETH). So the full impact is not correctly shown. At most valid low, since if you extend to 18 decimals precision, the possible precision loss is actually low.

#124 - Example given have no relation to affected code and hence is not valid.

#19, #142 - While precision loss is possible, both lack an explicit example to show impact of precision loss. Based on sherlocks guidelines, at most valid low

In case of issues related to precision loss, there must be a valid POC/example showing the loss to justify the medium/high severity.

@rcstanciu rcstanciu added the Has Duplicates A valid issue with 1+ other issues describing the same vulnerability label Nov 8, 2023
@sherlock-admin sherlock-admin changed the title Trendy Lilac Weasel - Precision Loss Due to Non-Exact Multiples in Division sil3th - Precision Loss Due to Non-Exact Multiples in Division Nov 9, 2023
@sherlock-admin sherlock-admin added Non-Reward This issue will not receive a payout and removed Has Duplicates A valid issue with 1+ other issues describing the same vulnerability labels Nov 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Excluded Excluded by the judge without consulting the protocol or the senior Non-Reward This issue will not receive a payout
Projects
None yet
Development

No branches or pull requests

4 participants