code-423n4 / 2023-05-maia-findings

24 stars 13 forks source link

_normalizeDecimals() Wrong calculation formula #790

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-05-maia/blob/54a45beb1428d85999da3f721f923cbf36ee3d35/src/ulysses-omnichain/BranchBridgeAgent.sol#L1341

Vulnerability details

Impact

Wrong decimal place conversion, resulting in wrong quantity

Proof of Concept

in callOutSignedAndBridge() The number of tokens will be converted to 18 decimal when packedData is performed.

    function callOutSignedAndBridge(bytes calldata _params, DepositInput memory _dParams, uint128 _remoteExecutionGas)
        external
        payable
        lock
        requiresFallbackGas
    {
        //Encode Data for cross-chain call.
        bytes memory packedData = abi.encodePacked(
            bytes1(0x05),
            msg.sender,
            depositNonce,
            _dParams.hToken,
            _dParams.token,
            _dParams.amount,
@>          _normalizeDecimals(_dParams.deposit, ERC20(_dParams.token).decimals()),
            _dParams.toChain,
            _params,
            msg.value.toUint128(),
            _remoteExecutionGas
        );

But the method of converting to 18 decimal is wrong

    /**
     * @notice Internal function that normalizes an input to 18 decimal places.
     * @param _amount amount of tokens
     * @param _decimals number of decimal places
     */
    function _normalizeDecimals(uint256 _amount, uint8 _decimals) internal pure returns (uint256) {
@>      return _decimals == 18 ? _amount : _amount * (10 ** _decimals) / 1 ether;
    }

The correct should be _amount * 1 ether / (10 ** _decimals)

Many places have problems using this method

Note: BranchPort._denormalizeDecimals() Have similar questions

Tools Used

Recommended Mitigation Steps

    /**
     * @notice Internal function that normalizes an input to 18 decimal places.
     * @param _amount amount of tokens
     * @param _decimals number of decimal places
     */
    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);
    }

Assessed type

Context

c4-judge commented 1 year ago

trust1995 marked the issue as duplicate of #758

c4-judge commented 1 year ago

trust1995 marked the issue as satisfactory

c4-judge commented 1 year ago

trust1995 marked the issue as partial-50