sherlock-audit / 2023-01-uxd-judging

3 stars 1 forks source link

serial-coder - [M-01] Loss Of UXD And UXP Tokens During Cross-Chain Bridging #354

Closed github-actions[bot] closed 1 year ago

github-actions[bot] commented 1 year ago

serial-coder

medium

[M-01] Loss Of UXD And UXP Tokens During Cross-Chain Bridging

Summary

The UXD and UXP are Omnichain Fungible Tokens (OFTs), allowing users to bridge the tokens from a source chain to a destination chain through the LayerZero protocol.

I found that the bridging-related contracts used by the UXD protocol to interact with the LayerZero protocol lack proper sanitization checks on the target address (_toAddress), the address that is the token receiver on the destination chain.

As a result, this vulnerability can cause users to lose their UXD and UXP tokens during the cross-chain bridging processes. Moreover, this vulnerability also causes the state inconsistency issue in the UXDToken contract.

Vulnerability Detail

The UXD protocol employs a set of bridging contracts, including LzApp, NonblockingLzApp, OFTCore, and OFT, to bridge UXD and UXP tokens from a source chain to a destination chain.

The below snippet illustrates an execution flow of bridging-related functions when a user on a source chain triggers the cross-chain token bridging process.

SNIPPET: 1
DESCRIPTION: An execution flow of the OFT token bridging on a source chain

srcChain:

    User -> OFTCore.sendFrom(
                address _from, 
                uint16 _dstChainId, 
                bytes calldata _toAddress, 
                uint _amount, 
                address payable _refundAddress, 
                address _zroPaymentAddress, 
                bytes calldata _adapterParams
            )

         -> OFTCore._send(
                address _from, 
                uint16 _dstChainId, 
                bytes memory _toAddress, 
                uint _amount, 
                address payable _refundAddress, 
                address _zroPaymentAddress, 
                bytes memory _adapterParams
            )

         -> OFT._debitFrom(
                address _from, 
                uint16 _dstChainId, 
                bytes memory _toAddress, 
                uint _amount
            )

         -> ERC20._burn(
                address account, 
                uint256 amount
            )

         -> LzApp._lzSend(
                uint16 _dstChainId, 
                bytes memory _payload, 
                address payable _refundAddress, 
                address _zroPaymentAddress, 
                bytes memory _adapterParams, 
                uint _nativeFee
            )

         -> LzApp._lzSend() triggers the relayer (lzEndpoint) of a destination chain

The snippet below shows the OFTCore.sendFrom() function (L31 - 33). This function is the entry point invoked by a user on a source chain.

The function parameter that is the root cause of this vulnerability is _toAddress, encoded in bytes.

Later, the OFTCore.sendFrom() function will execute the internal function OFTCore._send() (L32). The _send() function (L53 - 62) encodes the lzPayload (L58) by executing the following statement:

lzPayload = abi.encode(PT_SEND, _toAddress, amount);

As you can see, the _toAddress parameter is encoded into the lzPayload without validation. If the address(0) encoded in bytes is passed to the _toAddress parameter, the bridging-related contracts on a source chain would not be detectable.

Furthermore, the _send() function also invokes the OFT._debitFrom(_from, _dstChainId, _toAddress, _amount) function (L56). Finally, the _send() function will invoke the LzApp._lzSend() function (L59) to trigger the lzEndpoint, the relayer of a destination chain.

SNIPPET: 2
FILE: https://github.com/sherlock-audit/2023-01-uxd/blob/main/contracts/external/layer-zero/token/oft/OFTCore.sol
LOCATIONS: L32, L56, L58, and L59

31:     function sendFrom(address _from, uint16 _dstChainId, bytes calldata _toAddress, uint _amount, address payable _refundAddress, address _zroPaymentAddress, bytes calldata _adapterParams) public payable virtual override {
32: *       _send(_from, _dstChainId, _toAddress, _amount, _refundAddress, _zroPaymentAddress, _adapterParams);
33:     }

        // ...

53:     function _send(address _from, uint16 _dstChainId, bytes memory _toAddress, uint _amount, address payable _refundAddress, address _zroPaymentAddress, bytes memory _adapterParams) internal virtual {
54:         _checkAdapterParams(_dstChainId, PT_SEND, _adapterParams, NO_EXTRA_GAS);
55:
56: *       uint amount = _debitFrom(_from, _dstChainId, _toAddress, _amount);
57:
58: *       bytes memory lzPayload = abi.encode(PT_SEND, _toAddress, amount);
59: *       _lzSend(_dstChainId, lzPayload, _refundAddress, _zroPaymentAddress, _adapterParams, msg.value);
60:
61:         emit SendToChain(_dstChainId, _from, _toAddress, amount);
62:     }

The below snippet presents the OFT._debitFrom() function. This function will burn the user's OFT tokens (i.e., UXD and UXP) on a source chain (L29).

SNIPPET: 3
FILE: https://github.com/sherlock-audit/2023-01-uxd/blob/main/contracts/external/layer-zero/token/oft/OFT.sol
LOCATIONS: L29

26:     function _debitFrom(address _from, uint16, bytes memory, uint _amount) internal virtual override returns(uint) {
27:         address spender = _msgSender();
28:         if (_from != spender) _spendAllowance(_from, spender, _amount);
29: *       _burn(_from, _amount);
30:         return _amount;
31:     }

After the bridging transaction on a source chain is successfully executed, the lzEndpoint, a bridge relayer of the destination chain, will be triggered.

The lzEndpoint invokes the LzApp.lzReceive() function to initiate the OFT token minting process on the destination chain.

The below snippet depicts an execution flow of bridging-related functions triggered by the lzEndpoint on the destination chain.

SNIPPET: 4
DESCRIPTION: An execution flow of the OFT token bridging on a destination chain

destChain:

    lzEndpoint -> LzApp.lzReceive(
                        uint16 _srcChainId, 
                        bytes calldata _srcAddress, 
                        uint64 _nonce, 
                        bytes calldata _payload
                    )

               -> NonblockingLzApp._blockingLzReceive(
                        uint16 _srcChainId, 
                        bytes memory _srcAddress, 
                        uint64 _nonce, 
                        bytes memory _payload
                    )

               -> NonblockingLzApp.nonblockingLzReceive(
                        uint16 _srcChainId, 
                        bytes calldata _srcAddress, 
                        uint64 _nonce, 
                        bytes calldata _payload
                    )

               -> OFTCore._nonblockingLzReceive(
                        uint16 _srcChainId, 
                        bytes memory _srcAddress, 
                        uint64 _nonce, 
                        bytes memory _payload
                    )

               -> OFTCore._sendAck(
                        uint16 _srcChainId, 
                        bytes memory, uint64, 
                        bytes memory _payload
                    )

               -> OFT._creditTo(
                        uint16, 
                        address _toAddress, 
                        uint _amount
                    )

               -> ERC20._mint(
                        address account, 
                        uint256 amount
                    )

The below snippet shows the OFTCore._sendAck() function, one of the bridging-related functions on the destination chain.

The OFTCore._sendAck() function will decode the payload previously encoded at the source chain (L65 - 67). At this point, the target address (to) indicating the token receiver's address is decoded (L67).

Later, the OFT._creditTo(_srcChainId, to, amount) function will be executed (L69).

SNIPPET: 5
FILE: https://github.com/sherlock-audit/2023-01-uxd/blob/main/contracts/external/layer-zero/token/oft/OFTCore.sol
LOCATIONS: L65, L67, and L69

64:     function _sendAck(uint16 _srcChainId, bytes memory, uint64, bytes memory _payload) internal virtual {
65: *       (, bytes memory toAddressBytes, uint amount) = abi.decode(_payload, (uint16, bytes, uint));
66:
67: *       address to = toAddressBytes.toAddress(0);
68:
69: *       amount = _creditTo(_srcChainId, to, amount);
70:         emit ReceiveFromChain(_srcChainId, to, amount);
71:     }

The following snippet shows the OFT._creditTo() function. This function would invoke the ERC20._mint(_toAddress, _amount) function (L34) to mint the OFT tokens on the destination chain.

SNIPPET: 6
FILE: https://github.com/sherlock-audit/2023-01-uxd/blob/main/contracts/external/layer-zero/token/oft/OFT.sol
LOCATIONS: L34

33:     function _creditTo(uint16, address _toAddress, uint _amount) internal virtual override returns(uint) {
34: *       _mint(_toAddress, _amount);
35:         return _amount;
36:     }

The snippet below presents the ERC20._mint() function. At this point, the address(0) would be passed to the account parameter, making the bridging transaction on the destination chain revert in L258.

SNIPPET: 7
FILE: @openzeppelin/contracts/token/ERC20/ERC20.sol
LOCATIONS: L258

257:    function _mint(address account, uint256 amount) internal virtual {
258: *      require(account != address(0), "ERC20: mint to the zero address");
259:
260:        _beforeTokenTransfer(address(0), account, amount);
261:
262:        _totalSupply += amount;
263:        _balances[account] += amount;
264:        emit Transfer(address(0), account, amount);
265:
266:        _afterTokenTransfer(address(0), account, amount);
267:    }

Impact

This vulnerability causes the OFT token minting transaction to revert on a destination chain. In other words, the token receiver would not receive any token from the bridge. Whereas the OFT tokens on a source chain would be burned, making the token sender lose their tokens permanently.

Furthermore, since the UXP is the UXD protocol's governance token, the loss of UXP tokens can also cause a negative impact on the decentralization of the protocol.

Moreover, in the case of the UXD token bridging, I discovered that this vulnerability also causes the state inconsistency issue in the UXDToken contract. Specifically, since the UXD tokens on a source chain would be unexpectedly burned permanently, I noticed that the state variable localMintAmount of the UXDToken contract would not be updated (decreasing) as expected. This could eventually affect the UXD token mint accounting process.

Even if the likelihood is considered LOW, nevertheless, it is possible due to several factors, such as users' mistakes, UXD protocol's front-end or back-end errors, or errors from other protocols that utilize the UXD and UXP tokens.

For the impact of this vulnerability is considered HIGH. Therefore, the risk of this vulnerability is MEDIUM.

Code Snippet

https://github.com/sherlock-audit/2023-01-uxd/blob/main/contracts/external/layer-zero/token/oft/OFTCore.sol#L31-L33

https://github.com/sherlock-audit/2023-01-uxd/blob/main/contracts/external/layer-zero/token/oft/OFTCore.sol#L53-L62

https://github.com/sherlock-audit/2023-01-uxd/blob/main/contracts/external/layer-zero/token/oft/OFT.sol#L26-L31

https://github.com/sherlock-audit/2023-01-uxd/blob/main/contracts/external/layer-zero/token/oft/OFTCore.sol#L64-L71

https://github.com/sherlock-audit/2023-01-uxd/blob/main/contracts/external/layer-zero/token/oft/OFT.sol#L33-L36

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/6a8d977d2248cf1c115497fccfd7a2da3f86a58f/contracts/token/ERC20/ERC20.sol#L260

Tool used

Manual Review

Recommendation

I recommend adding a sanitization check in the OFTCore.sendFrom() function (the entry function on the source chain) like L32 - 33 in the snippet below.

SNIPPET: 8
FILE: https://github.com/sherlock-audit/2023-01-uxd/blob/main/contracts/external/layer-zero/token/oft/OFTCore.sol
LOCATIONS: L32 - 33

31:     function sendFrom(address _from, uint16 _dstChainId, bytes calldata _toAddress, uint _amount, address payable _refundAddress, address _zroPaymentAddress, bytes calldata _adapterParams) public payable virtual override {
32: *       address to = _toAddress.toAddress(0);
33: *       require(to != address(0), "LzApp: send to the zero address");
34:
35:         _send(_from, _dstChainId, _toAddress, _amount, _refundAddress, _zroPaymentAddress, _adapterParams);
36:     }
hrishibhat commented 1 year ago

Zero address check for the encoded input value in this case is not considered a valid medium.