code-423n4 / 2021-10-tempus-findings

0 stars 0 forks source link

Scaling factors for token 0/1 might swap in TempusAMM constructor. #7

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

chenyu

Vulnerability details

Impact

In TempusAMM constructor L138, the scaling factor 0 always maps to yieldShare, and scaling factor 1 always maps to principalShare, even though in L134 the two token might swap if principalShare < yieldShare, which makes _token0 = principalShare and _token1 = yieldShare, but scaling factor 0 is based on yieldShare.

Later _scalingFactor is based on _token0, so it might get the wrong scaling factor if principalShare and yieldShare had swapped.

Recommended Mitigation Steps

Update the lines to

        _scalingFactor0 = _computeScalingFactor(IERC20(address(_token0)));
        _scalingFactor1 = _computeScalingFactor(IERC20(address(_token1)));
mijovic commented 3 years ago

While this works with Tempus Principals and Tempus Yields (have the same scaling factor), AMM was made to be able to be reused for some other tokens with similar correlation, so this must be fixed.

mijovic commented 2 years ago

Fixed in https://github.com/tempus-finance/tempus-protocol/pull/359