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

24 stars 13 forks source link

_createDepositSingle() call bridgeOut missing normalizeDecimals #802

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

Wrong decimal place conversion, resulting in wrong quantity

Proof of Concept

in _createDepositSingle() will call IPort(localPortAddress).bridgeOut() The parameter _deposit is not converted to 18 decimal

_createDepositSingle()

    function _createDepositSingle(
        address _user,
        address _hToken,
        address _token,
        uint256 _amount,
        uint256 _deposit,
        uint128 _gasToBridgeOut
    ) internal {
        //Deposit / Lock Tokens into Port
@>      IPort(localPortAddress).bridgeOut(_user, _hToken, _token, _amount, _deposit);

        //Deposit Gas to Port
        _depositGas(_gasToBridgeOut);
...

but in IPort(localPortAddress).bridgeOut() method ,will do _denormalizeDecimals on deposit from 18 decimal to token.decimals

    function bridgeOut(
        address _depositor,
        address _localAddress,
        address _underlyingAddress,
        uint256 _amount,
        uint256 _deposit
    ) external virtual requiresBridgeAgent {
        if (_amount - _deposit > 0) {
            _localAddress.safeTransferFrom(_depositor, address(this), _amount - _deposit);
            ERC20hTokenBranch(_localAddress).burn(_amount - _deposit);
        }
        if (_deposit > 0) {
            _underlyingAddress.safeTransferFrom(
@>              _depositor, address(this), _denormalizeDecimals(_deposit, ERC20(_underlyingAddress).decimals())
            );
        }
    }

So we need to convert deposit to 18 decimal before calling bridgeOut

Note: _createDepositMultiple have similar questions

Tools Used

Recommended Mitigation Steps

    function _createDepositSingle(
        address _user,
        address _hToken,
        address _token,
        uint256 _amount,
        uint256 _deposit,
        uint128 _gasToBridgeOut
    ) internal {
        //Deposit / Lock Tokens into Port
-      IPort(localPortAddress).bridgeOut(_user, _hToken, _token, _amount, _deposit);        
+      IPort(localPortAddress).bridgeOut(_user, _hToken, _token, _amount, _normalizeDecimals(_deposit));

        //Deposit Gas to Port
        _depositGas(_gasToBridgeOut);
...

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