code-423n4 / 2024-03-saltyio-mitigation-findings

0 stars 0 forks source link

M-25 MitigationConfirmed #106

Open c4-bot-8 opened 4 months ago

c4-bot-8 commented 4 months ago

Lines of code

Vulnerability details

Lines of code

Vulnerability details

C4 Issue

https://github.com/code-423n4/2024-01-salty-findings/issues/232

Comments

The issue describes how due to an incorrect assumption in PoolMath.sol, an underflow situation can occur when zapping is used, which triggers a revert. The issue lies specifically in this line:

        uint256 C = r0 * ( r1 * z0 - r0 * z1 ) / ( r1 + z1 );

to avoid overflow, r0, r1, z0 and z1 are shifted downwards. if the shift is so big such that z0 or r1 is shifted down to equal 0, then the calculation above will underflow.

Mitigation

https://github.com/othernet-global/salty-io/commit/44320a8cc9b94de433e437e025f072aa850b995a

The mitigation for this issue revolves around completely removing the scaling down of the reserve and zap amounts. Given that scaling was removed, the calculation for C was modified to ensure division happens before the multiplication, to reduce the risk of overflow:

uint256 C = r0 * ( ( r1 * z0 - r0 * z1 ) / ( r1 + z1 ) );

Keep in mind the risk of overflow is still inherently there especially after removing the scaling, but after review unlikely given the largest multiplicative calculation in the function is (2*r0)^2. you would need an astronomically large reserve0 value for risks of overflow to show up.

Something to keep in mind also is the issue of loss of precision due to performing division before multiplication, which will reduce the zap swap amount, but impact is minimal.

Conclusion

LGTM

c4-judge commented 4 months ago

Picodes marked the issue as satisfactory

c4-judge commented 4 months ago

Picodes marked the issue as confirmed for report