Divide before multiply

[code423n4]

Lines of code

https://github.com/code-423n4/2022-09-quickswap/blob/2ead456d3603d8a4d839cf88f1e41c102b5d040f/src/core/contracts/libraries/AdaptiveFee.sol#L70-L108 https://github.com/code-423n4/2022-09-quickswap/blob/2ead456d3603d8a4d839cf88f1e41c102b5d040f/src/core/contracts/libraries/AdaptiveFee.sol#L70-L108 https://github.com/code-423n4/2022-09-quickswap/blob/2ead456d3603d8a4d839cf88f1e41c102b5d040f/src/core/contracts/libraries/AdaptiveFee.sol#L70-L108 https://github.com/code-423n4/2022-09-quickswap/blob/2ead456d3603d8a4d839cf88f1e41c102b5d040f/src/core/contracts/libraries/AdaptiveFee.sol#L70-L108 https://github.com/code-423n4/2022-09-quickswap/blob/2ead456d3603d8a4d839cf88f1e41c102b5d040f/src/core/contracts/libraries/DataStorage.sol#L205-L263 https://github.com/code-423n4/2022-09-quickswap/blob/2ead456d3603d8a4d839cf88f1e41c102b5d040f/src/core/contracts/libraries/AdaptiveFee.sol#L70-L108 https://github.com/code-423n4/2022-09-quickswap/blob/2ead456d3603d8a4d839cf88f1e41c102b5d040f/src/core/contracts/libraries/DataStorage.sol#L205-L263 https://github.com/code-423n4/2022-09-quickswap/blob/2ead456d3603d8a4d839cf88f1e41c102b5d040f/src/core/contracts/libraries/AdaptiveFee.sol#L70-L108

Vulnerability details

Divide before multiply

Impact

Solidity integer division might truncate. As a result, performing multiplication before division can sometimes avoid loss of precision.

Details

In general, this is a problem due to precision. In this case, it also affects money/tokens, what makes me suggest medium severity.

beforeOrAt.volatilityCumulative += ((atOrAfter.volatilityCumulative - beforeOrAt.volatilityCumulative) / timepointTimeDelta) targetDelta https://github.com/code-423n4/2022-09-quickswap/blob/2ead456d3603d8a4d839cf88f1e41c102b5d040f/src/core/contracts/libraries/DataStorage.sol#L205-L263 gHighestDegree /= g res += (xLowestDegree gHighestDegree) / 6

https://github.com/code-423n4/2022-09-quickswap/blob/2ead456d3603d8a4d839cf88f1e41c102b5d040f/src/core/contracts/libraries/AdaptiveFee.sol#L70-L108 gHighestDegree /= g res += (xLowestDegree * gHighestDegree) / 120

https://github.com/code-423n4/2022-09-quickswap/blob/2ead456d3603d8a4d839cf88f1e41c102b5d040f/src/core/contracts/libraries/AdaptiveFee.sol#L70-L108

beforeOrAt.volumePerLiquidityCumulative += ((atOrAfter.volumePerLiquidityCumulative - beforeOrAt.volumePerLiquidityCumulative) / timepointTimeDelta) * targetDelta

https://github.com/code-423n4/2022-09-quickswap/blob/2ead456d3603d8a4d839cf88f1e41c102b5d040f/src/core/contracts/libraries/DataStorage.sol#L205-L263

gHighestDegree /= g res += (xLowestDegree * gHighestDegree) / 2

https://github.com/code-423n4/2022-09-quickswap/blob/2ead456d3603d8a4d839cf88f1e41c102b5d040f/src/core/contracts/libraries/AdaptiveFee.sol#L70-L108

gHighestDegree /= g res += (xLowestDegree * gHighestDegree) / 720

https://github.com/code-423n4/2022-09-quickswap/blob/2ead456d3603d8a4d839cf88f1e41c102b5d040f/src/core/contracts/libraries/AdaptiveFee.sol#L70-L108 beforeOrAt.tickCumulative += ((atOrAfter.tickCumulative - beforeOrAt.tickCumulative) / timepointTimeDelta) * targetDelta

https://github.com/code-423n4/2022-09-quickswap/blob/2ead456d3603d8a4d839cf88f1e41c102b5d040f/src/core/contracts/libraries/DataStorage.sol#L205-L263 gHighestDegree /= g res += (xLowestDegree * gHighestDegree) / 24

https://github.com/code-423n4/2022-09-quickswap/blob/2ead456d3603d8a4d839cf88f1e41c102b5d040f/src/core/contracts/libraries/AdaptiveFee.sol#L70-L108

gHighestDegree /= g res += xLowestDegree * gHighestDegree

https://github.com/code-423n4/2022-09-quickswap/blob/2ead456d3603d8a4d839cf88f1e41c102b5d040f/src/core/contracts/libraries/AdaptiveFee.sol#L70-L108

Tools

Slither + manual analysis

Proof of Concept

Lack of precision due to the order of operations

Recommended Mitigation Steps

Reorder the operations. For more info: https://github.com/crytic/slither/wiki/Detector-Documentation#divide-before-multiply

[0xVolosnikov]

Please note that in all cases the dividend is divisible by the divisor by design. Except for dividing by constants here: https://github.com/code-423n4/2022-09-quickswap/blob/2ead456d3603d8a4d839cf88f1e41c102b5d040f/src/core/contracts/libraries/AdaptiveFee.sol#L70-L108

[0xean]

downgrading to QA and duplicate of #313 - the wardens QA report.