code-423n4 / 2023-02-kuma-findings

2 stars 1 forks source link

Unnecessary precision loss in redeemKIBT() #5

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-02-kuma/blob/3f3d2269fcb3437a9f00ffdd67b5029487435b95/src/kuma-protocol/KUMASwap.sol#L309-L310

Vulnerability details

Impact

Unnecessary precision loss in redeemKIBT()

Proof of Concept

If enter Deprecated mode, user can redeem StableCoin by percentage with redeemKIBT()

The redeemKIBT() implementation code is as follows:

    function redeemKIBT(uint256 amount) external override whenDeprecated {
        if (amount == 0) {
            revert Errors.AMOUNT_CANNOT_BE_ZERO();
        }
        if (_bondReserve.length() != 0) {
            revert Errors.BOND_RESERVE_NOT_EMPTY();
        }
        IKIBToken KIBToken = IKIBToken(_KUMAAddressProvider.getKIBToken(_riskCategory));
        IERC20 deprecationStableCoin = _deprecationStableCoin;

        uint256 redeemAmount =
            amount.wadMul(_deprecationStableCoin.balanceOf(address(this))).wadDiv(KIBToken.totalSupply());
        KIBToken.burn(msg.sender, amount);
        deprecationStableCoin.safeTransfer(msg.sender, redeemAmount);

        emit KIBTRedeemed(msg.sender, redeemAmount);
    }

Simplify the formula:

redeemAmount = (amount * StableCoinBalance / 10**18 ) * 10**18 / totalSupply

The above formula amount * StableCoinBalance / 10**18 maybe have the possibility of precision loss and StableCoinBalance as usdc's decimals == 6

It is recommended to use redeemAmount = amount * StableCoinBalance / totalSupply

Test code:

  function test_precision() external {
      uint256 stableCoin = 1_000_000;
      uint256 totalSupply = 10_000_000_000;
      uint256 amount = totalSupply;        
      uint256 old = (amount * stableCoin/10**18) * 10**18 / totalSupply;
      uint256 new_ = amount * stableCoin / totalSupply;
      console.log("old formula result :",old);
      console.log("new formula result:",new_);
  }
forge test --match test_precision -vvv

[PASS] test_precision() (gas: 4632)
Logs:
  old formula result : 0
  new formula result : 1000000

Test result: ok. 1 passed; 0 failed; finished in 917.24µs

Tools Used

Recommended Mitigation Steps

    function redeemKIBT(uint256 amount) external override whenDeprecated {
        if (amount == 0) {
            revert Errors.AMOUNT_CANNOT_BE_ZERO();
        }
        if (_bondReserve.length() != 0) {
            revert Errors.BOND_RESERVE_NOT_EMPTY();
        }
        IKIBToken KIBToken = IKIBToken(_KUMAAddressProvider.getKIBToken(_riskCategory));
        IERC20 deprecationStableCoin = _deprecationStableCoin;

        uint256 redeemAmount =
-           amount.wadMul(_deprecationStableCoin.balanceOf(address(this))).wadDiv(KIBToken.totalSupply());
+           amount * _deprecationStableCoin.balanceOf(address(this)) / KIBToken.totalSupply();
        KIBToken.burn(msg.sender, amount);
        deprecationStableCoin.safeTransfer(msg.sender, redeemAmount);

        emit KIBTRedeemed(msg.sender, redeemAmount);
    }
GalloDaSballo commented 1 year ago

Valid report, but impact shown is up to 1Dollar

@bin2chen66 please do send an instance with higher impact if you can generate it

If impact is at that size I will downgrade to QA Low

bin2chen66 commented 1 year ago

@GalloDaSballo Tried and could not generate Agree with you

c4-judge commented 1 year ago

GalloDaSballo changed the severity to QA (Quality Assurance)

GalloDaSballo commented 1 year ago

L

c4-sponsor commented 1 year ago

m19 marked the issue as sponsor confirmed

m19 commented 1 year ago

We confirm this issue but we agree with the judge that the impact is very low and thus should be considered QA/low.

c4-judge commented 1 year ago

GalloDaSballo marked the issue as grade-a