Normalizing decimals to 18 decimals returns the amount scaled up by a magnitude different than 18 when the original tokenDecimals are not 18 decimals

[code423n4]

Lines of code

https://github.com/code-423n4/2023-05-maia/blob/main/src/ulysses-omnichain/BranchBridgeAgent.sol#L1335-L1342

Vulnerability details

Impact

• Incorrectly computing the magnitude of the token amounts will make that users transfer more tokens than what they really expect to transfer
• Also is possible that for a good number of users it becomes impossible to even cover the wrong computed amount of tokens, thus, they won't be able to use the systems
• Also using an incorrect magnitude can impact the internal accounting and balances in the system, which could lead to losses or even DoS in functions dependant on the internal accounting & balances.

Proof of Concept

• The current formula to normalize the decimals is the below one:

  function _normalizeDecimals(uint256 _amount, uint8 _decimals) internal pure returns (uint256) {
  return _decimals == 18 ? _amount : _amount * (10 ** _decimals) / 1 ether;
  }

• By doing the math we have:

  • Example 1: A token that has 8 decimals:
    • _amount is scaled up by the tokensDecimals!
    • The formula: _amount * (10 ** _decimals) / 1 ether
    • Substituting values: [(10**8) * (10**8)] / (10**18) ===> (10**16) / (10**18) ===> 10**-2
    • result: This doesnt even compile in solidity, it will throw an error as follows:
      • TypeError: Return argument type rational_const 1 / 100 is not implicitly convertible to expected type (type of first return variable) uint256.

  • Example 2: A token that has 30 decimals:

    • _amount is scaled up by the tokensDecimals!

    • The formula: _amount * (10 ** _decimals) / 1 ether

    • Substituting values: [(10**30) * (10**30)] / (10**18) ===> (10**60) / (10**18) ===> 10**42

    • result: 10**42

• As demonstrated in the above examples, the current formula doesn't compute correctly the normalization of the decimals to 18 decimals.

Tools Used

Manual Audit

Recommended Mitigation Steps

• Make sure to update the formula to normalize the decimals to 18 as follows:

  
  function _normalizeDecimals(uint256 _amount, uint8 _decimals) internal pure returns (uint256) {

• return _decimals == 18 ? _amount : _amount * (10 ** _decimals) / 1 ether;

• return _decimals == 18 ? _amount : _amount * 1 ether / (10 ** _decimals); }

• By doing the math we have:

  • Example 1: A token that has 8 decimals:

    • _amount is scaled up by the tokensDecimals!

    • The formula: _amount * 1 ether / (10 ** _decimals)

    • Substituting values: [(10**8) * (10**18)] / (10**8) ===> (10**26) / (10**8) ===> 10**18

    • result: 10**18

  • Example 2: A token that has 30 decimals:

    • _amount is scaled up by the tokensDecimals!

    • The formula: _amount * 1 ether / (10 ** _decimals)

    • Substituting values: [(10**30) * (10**18)] / (10**30) ===> (10**48) / (10**30) ===> 10**18

    • result: 10**18

Assessed type

Math

[c4-judge]

trust1995 marked the issue as duplicate of #758

[c4-judge]

trust1995 marked the issue as satisfactory

[c4-judge]

trust1995 marked the issue as partial-50