Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SwapUtils.sol Wrong implementation #252

Open
code423n4 opened this issue Nov 10, 2021 · 0 comments
Open

SwapUtils.sol Wrong implementation #252

code423n4 opened this issue Nov 10, 2021 · 0 comments
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")

Comments

@code423n4
Copy link
Contributor

Handle

WatchPug

Vulnerability details

Based on the context, the tokenPrecisionMultipliers used in price calculation should be calculated in realtime based on initialTargetPrice, futureTargetPrice, futureTargetPriceTime and current time, just like getA() and getA2().

However, in the current implementation, tokenPrecisionMultipliers used in price calculation is the stored value, it will only be changed when the owner called rampTargetPrice() and stopRampTargetPrice().

As a result, the targetPrice set by the owner will not be effective until another targetPrice is being set or stopRampTargetPrice() is called.

Recommendation

Consider adding Swap.targetPrice and changing the _xp() at L661 from:

https://github.com/code-423n4/2021-11-bootfinance/blob/f102ee73eb320532c5a7c1e833f225c479577e39/customswap/contracts/SwapUtils.sol#L661-L667

function _xp(Swap storage self, uint256[] memory balances)
    internal
    view
    returns (uint256[] memory)
{
    return _xp(balances, self.tokenPrecisionMultipliers);
}

To:

function _xp(Swap storage self, uint256[] memory balances)
    internal
    view
    returns (uint256[] memory)
{
    uint256[2] memory tokenPrecisionMultipliers = self.tokenPrecisionMultipliers;
    tokenPrecisionMultipliers[0] = self.targetPrice.originalPrecisionMultipliers[0].mul(_getTargetPricePrecise(self)).div(WEI_UNIT)
    return _xp(balances, tokenPrecisionMultipliers);
}
@code423n4 code423n4 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Nov 10, 2021
code423n4 added a commit that referenced this issue Nov 10, 2021
@chickenpie347 chickenpie347 added the sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") label Jan 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")
Projects
None yet
Development

No branches or pull requests

2 participants