sherlock-audit / 2022-11-bond-judging

1 stars 1 forks source link

xiaoming90 - Rounding Issue In Control Variable #17

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

xiaoming90

medium

Rounding Issue In Control Variable

Summary

The rounding error when computing the control variable causes the control variable to be lower, leading to the makers selling tokens at a lower price than expected, as the market price of a token is computed as a product of the control variable and debt.

Vulnerability Detail

The computed control variable at Line 600 is rounded up to achieve the desirable property that the integer implementation of the control variable will be greater than or equal to the real value of the control variable. This ensures that the integer implementation of the price calculated from controlVariable * debt will be greater than or equal to the real value of the price, which protects makers from selling tokens at a lower price than expected.

https://github.com/sherlock-audit/2022-11-bond/blob/main/src/bases/BondBaseSDA.sol#L598

File: BondBaseSDA.sol
598:             // Derive a new control variable from the target debt
599:             uint256 controlVariable = terms[id_].controlVariable;
600:             uint256 newControlVariable = price_.mulDivUp(market.scale, targetDebt);

However, this is not consistently applied throughout the codebase. In the following code, the control variable is rounded down, which will result in the integer implementation of the control variable to be lower than the real value of the control variable. This, in turn, leads to the makers selling tokens at a lower price than expected.

https://github.com/sherlock-audit/2022-11-bond/blob/main/src/bases/BondBaseSDA.sol#L280

File: BondBaseSDA.sol
280:         // price = control variable * debt / scale
281:         // therefore, control variable = price * scale / debt
282:         uint256 controlVariable = params_.formattedInitialPrice.mulDiv(scale, targetDebt);

Impact

The market price of a token is computed as a product of the control variable and debt. If the control variable is lower than expected, the tokens will be sold at a lower price than expected, leading to a loss for the market makers.

Code Snippet

https://github.com/sherlock-audit/2022-11-bond/blob/main/src/bases/BondBaseSDA.sol#L280

https://github.com/sherlock-audit/2022-11-bond/blob/main/src/bases/BondBaseSDA.sol#L598

Tool used

Manual Review

Recommendation

Ensure that the rounding is consistently applied throughout the codebase when computing the control variable so that the desired property can be achieved.

- uint256 controlVariable = params_.formattedInitialPrice.mulDiv(scale, targetDebt);
+ uint256 controlVariable = params_.formattedInitialPrice.mulDivUp(scale, targetDebt);
Evert0x commented 1 year ago

Message from sponsor


Disagree with this issue. The rounding is implemented as specified in the SDAM paper. Specifically, controlVariable is initialized with a floor rounding operation as described in section 4.1.2, definition 19 to ensure that the price after market creation matches the value provided by the market creator. Separately, the newControlVariable value on a tune() is calculated with a ceiling rounding operation to ensure the new value is greater than or equal to the model value as described in section 4.2.10, definition 29. Therefore, price is greater than or equal to the model value.