Topmark - Precision Lose Possibility

[sherlock-admin2]

Topmark

medium

Precision Lose Possibility

Summary

Precision Lose Possibility

Vulnerability Detail

priceChange calculation of Line 73 of BoundedStepwiseExponentialPriceAdapter.sol would give error if a negative integer result is gotten. Since priceChange could be on the positive or negative side, this would lead to a precision error in next series of calculations or priceChange usage

Impact

From the calculation @ https://github.com/sherlock-audit/2023-06-Index/blob/main/index-protocol/contracts/protocol/integration/auction-price/BoundedStepwiseExponentialPriceAdapter.sol#L73 , using uint256 priceChange would cause precision loss if the priceChange is negative

Code Snippet

Tool used

Manual Review

Recommendation

int256 should be used instead of unint256 for priceChange Variable

[Shogoki]

Escalate I don’t think this is a valid duplicate of #1. The issue is not talking about overflow.

[sherlock-admin2]

Escalate I don’t think this is a valid duplicate of #1. The issue is not talking about overflow.

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[pblivin0x]

Invalid, in the listed code, it is impossible for that to evaluate negative.

If possible, i kindly request a POC from @Shogoki

[hrishibhat]

@pblivin0x I think the escalator here is saying that this not a duplicate of #1. The escalation is not validating the issue. Just disagreeing with the Escalation. But based on your comment this seems to be an invalid issue.

[pblivin0x]

@pblivin0x I think the escalator here is saying that this not a duplicate of #1. The escalation is not validating the issue. Just disagreeing with the Escalation. But based on your comment this seems to be an invalid issue.

Thank you @hrishibhat , I agree that issue is not a duplicate of #1 . I disagree with the listed vulnerability

priceChange calculation of Line 73 of BoundedStepwiseExponentialPriceAdapter.sol would give error if a negative integer result is gotten.

Because in the submitted code uint256 priceChange = scalingFactor * expExpression - WAD;, we have that scalingFactor >= 1 and expExpression >= WAD

I will note that in our remediations, we have replaced this line with

uint256 priceChange = FixedPointMathLib.mulWad(scalingFactor, expExpression - WAD);

[hrishibhat]

@pblivin0x Apologies , in the above comment i meant to say "Just disagreeing with the Escalation duplication" Either way, just confirming:

• This issue is not a duplicate of #1
• This issue is invalid as priceChange would never be negative.

Correct?

[pblivin0x]

@pblivin0x Apologies , in the above comment i meant to say "Just disagreeing with the ~Escalation~ duplication" Either way, just confirming:

• This issue is not a duplicate of kutugu - int256 type conversion may overflow, resulting in a price calculation error #1
• This issue is invalid as priceChange would never be negative.

Correct?

Correct, thank you!

[pblivin0x]

Adding references

scalingFactor >= 1 https://github.com/sherlock-audit/2023-06-Index/blob/8d348ed344635a068d458aa04956f966b6d3d4f3/index-protocol/contracts/protocol/integration/auction-price/BoundedStepwiseExponentialPriceAdapter.sol#L140

expExpression >= WAD https://github.com/Vectorized/solady/blob/f60377c261f33d2099fc795d6dc9b34c6dbdb2d7/test/FixedPointMathLib.t.sol#L18

[Topmark1]

but I feel L61 - int256 expArgument = int256(timeCoefficient * timeBucket); affects the validity of this - expExpression >= WAD

[Topmark1]

not withstanding, Wad is 1e18, scalingFactor * expExpression can be greater than zero but not necessarily greater than Wad, negativity is a possibility. But i feel the duplicate error is what is responsible for the possibility of the vulnerability in the first place

[pblivin0x]

1) timeCoefficent > 0 and timeBucket >= 0 implies that expArgument >= 0 2) expArgument >= 0 implies that expExpression >= WAD 3) scalingFactor >= 1 and expExpression >= WAD implies that scalingFactor * expExpression >= WAD

please provide counter example to any of these lines @Topmark1

[hrishibhat]

Result: Invalid Unique Considering this a non-issue based on the above comments

[sherlock-admin2]

Escalations have been resolved successfully!

Escalation status:

• Shogoki: accepted