Digital garden

code4rena-2023-12-shell-protocol-l08

6 min read

code4rena-2023-12-shell-protocol-l08

[L-08] Extreme edge case error results in user paying more fees than usual when user decides to mint little shTokens with low decimals tokens

보고서

Summary

유저가 소수점이 낮은 토큰을 wrap 하고, 적은 양을 wrap할 때 더 많은 수수료를 지불하게 된다.

Keyword

token decimals, arithmetic error, rounding error

Vulnerability

낮은 decimal 토큰으로 warp 할 시 이슈가 발생할 수 있다.

2 decimal 을 사용하는 토큰 X가 있다고 가정하자. 토큰 하나(1e2)의 가치는 20000 USDC 이다. 이 때 유저가 1e15 개의 Ocean 토큰으로 민팅(wrap)하고 싶다.

  • convertDecimals(18, 2, 1e15) ⇒ transferAmount = 0, truncated = 1000000000000000
    • 1e15(18 decimal)를 2 decimal로 변경한다면 나눗셈에 의해 잘리는 값 truncatedAmount 가 존재
  • truncated > 0 이기 때문에, truncated > 0 조건문이 실행된다.
    • transferAmount += 1 로 잘린 값을 올림 처리했을 때 18 decimal에서의
    • converDecimals(2, 18, 1) ⇒ normalizedTransferAmount = 10000000000000000, normalizedTruncatedAmount = 0
      • 1e16
    • dust = normalizedTransferAmount - 1e15 = 9e15
    function _erc20Wrap(address tokenAddress, uint256 amount, address userAddress, uint256 outputToken) private {
        try IERC20Metadata(tokenAddress).decimals() returns (uint8 decimals) {
            /// @dev the amount passed as an argument to the external token
            uint256 transferAmount;
            /// @dev the leftover amount accumulated by the Ocean.
            uint256 dust;
 
@>          (transferAmount, dust) = _determineTransferAmount(amount, decimals);
 
            // If the user is unwrapping a delta, the residual dust could be
            // written to the user's ledger balance. However, it costs the
            // same amount of gas to place the dust on the owner's balance,
            // and accumulation of dust may eventually result in
            // transferrable units again.
@>          _grantFeeToOcean(outputToken, dust);
 
            SafeERC20.safeTransferFrom(IERC20(tokenAddress), userAddress, address(this), transferAmount);
 
            emit Erc20Wrap(tokenAddress, transferAmount, amount, dust, userAddress, outputToken);
        } catch {
            revert NO_DECIMAL_METHOD();
        }
    }
 
    function _determineTransferAmount(
        uint256 amount,
        uint8 decimals
    )
        private
        pure
        returns (uint256 transferAmount, uint256 dust)
    {
        // if (decimals < 18), then converting 18-decimal amount to decimals
        // transferAmount will likely result in amount being truncated. This
        // case is most likely to occur when a user is wrapping a delta as the
        // final interaction in a transaction.
        uint256 truncated;
 
@>      (transferAmount, truncated) = _convertDecimals(NORMALIZED_DECIMALS, decimals, amount);
 
@>      if (truncated > 0) {
            // Here, FLOORish(x) is not to the nearest integer less than `x`,
            // but rather to the nearest value with `decimals` precision less
            // than `x`. Likewise with CEILish(x).
            // When truncating, transferAmount is FLOORish(amount), but to
            // fully cover a potential delta, we need to transfer CEILish(amount)
            // if truncated == 0, FLOORish(amount) == CEILish(amount)
            // When truncated > 0, FLOORish(amount) + 1 == CEILish(AMOUNT)
@>          transferAmount += 1;
            // Now that we are transferring enough to cover the delta, we
            // need to determine how much of the token the user is actually
            // wrapping, in terms of 18-decimals.
            (uint256 normalizedTransferAmount, uint256 normalizedTruncatedAmount) =
@>              _convertDecimals(decimals, NORMALIZED_DECIMALS, transferAmount);
            // If we truncated earlier, converting the other direction is adding
            // precision, which cannot truncate.
            assert(normalizedTruncatedAmount == 0);
            assert(normalizedTransferAmount > amount);
@>          dust = normalizedTransferAmount - amount;
        } else {
            // if truncated == 0, then we don't need to do anything fancy to
            // determine transferAmount, the result _convertDecimals() returns
            // is correct.
            dust = 0;
        }
    }
 
    function _convertDecimals(
        uint8 decimalsFrom,
        uint8 decimalsTo,
        uint256 amountToConvert
    )
        internal
        pure
        returns (uint256 convertedAmount, uint256 truncatedAmount)
    {
        if (decimalsFrom == decimalsTo) {
            // no shift
            convertedAmount = amountToConvert;
            truncatedAmount = 0;
        } else if (decimalsFrom < decimalsTo) {
            // Decimal shift left (add precision)
            uint256 shift = 10 ** (uint256(decimalsTo - decimalsFrom));
            convertedAmount = amountToConvert * shift;
            truncatedAmount = 0;
        } else { // @audit-info 기존 decimal이 to보다 작은 경우
            // Decimal shift right (remove precision) -> truncation
            uint256 shift = 10 ** (uint256(decimalsFrom - decimalsTo));
            convertedAmount = amountToConvert / shift;
@>          truncatedAmount = amountToConvert % shift;
        }
    }

1e15 만큼의 Ocean 토큰을 발행하려면 1 wei 토큰을 지불해야 한다.(transferAmount) 사실 1 wei의 토큰의 가치는 1e15 Ocean 토큰보다 약간 높다. 하지만 이 약간 높은 가치는 토큰을 unwrap하며 2 decimal로 변환했을 때 내림당해 무시될 수 있는 양이다. 따라서 이 약간 높은 가치(9e15, dust)는 유저에게 주지 않고, 프로토콜의 수수료로 사용하여 프로토콜측에 민팅한다.

dust는 변환하고자 하는 ERC20의 decimal이 작을수록 커진다. 또한 소액을 민팅했을 때, 비율적으로 본다면 상대적으로 많은 비율의 수수료를 떼어간다.

Impact

유저가 소수점이 낮은 토큰을 wrap 하고, 적은 양을 wrap할 때 더 많은 수수료를 지불하게 된다.

Mitigation

작은 양의 Ocean 토큰을 발행할 수 없게 한다. 아주 작은 decimal의 토큰은 사용을 허용하지 않는다.

Memo

Medium로 제출, Low로 내려간 이슈이다. 프로토콜 측에서 6 decimal 미만의 토큰은 사용하지 않을 계획이라고 하여 Low로 친 것 같다.

tags: bughunting, shell protocol, smart contract, solidity, token decimals, arithmetic error, rounding error, severity low

Graph View