Attacker can block LayerZero channel

[code423n4]

Lines of code

https://github.com/code-423n4/2022-10-holograph/blob/main/contracts/HolographOperator.sol#L582-L652
https://github.com/code-423n4/2022-10-holograph/blob/main/contracts/module/LayerZeroModule.sol#L180-L222
https://github.com/code-423n4/2022-10-holograph/blob/main/contracts/HolographBridge.sol#L245-L283
https://github.com/code-423n4/2022-10-holograph/blob/main/contracts/HolographOperator.sol#L484-L539

Vulnerability details

Impact

According to the LayerZero docs, the default behavior is that when a transaction on the destination application fails, the channel between the src and dst app is blocked. Before any new transactions can be executed, the failed transaction has to be retried until it succeeds.

See https://layerzero.gitbook.io/docs/faq/messaging-properties#message-ordering & https://layerzero.gitbook.io/docs/guides/advanced/nonblockinglzapp

The retry logic has to be handled by the app itself. The current implementation of the LayerZeroModule contract doesn't have that. Thus, if the channel gets blocked, it will be blocked forever. There's no logic to retry any failed tx.

I've submitted the same bug in a previous contest: code-423n4/2022-05-velodrome-findings#83

Proof of Concept

To initiate a message from src to dst chain you call HolographBridge.bridgeOutRequest(). The function is permissionless:

function bridgeOutRequest(
    uint32 toChain,
    address holographableContract,
    uint256 gasLimit,
    uint256 gasPrice,
    bytes calldata bridgeOutPayload
  ) external payable {
    /**
     * @dev check that the target contract is either Holograph Factory or a deployed holographable contract
     */
    require(
      _registry().isHolographedContract(holographableContract) || address(_factory()) == holographableContract,
      "HOLOGRAPH: not holographed"
    );
    /**
     * @dev make a bridgeOut function call to the holographable contract
     */
    (bytes4 selector, bytes memory returnedPayload) = Holographable(holographableContract).bridgeOut(
      toChain,
      msg.sender,
      bridgeOutPayload
    );
    /**
     * @dev ensure returned selector is bridgeOut function signature, to guarantee that the function was called and succeeded
     */
    require(selector == Holographable.bridgeOut.selector, "HOLOGRAPH: bridge out failed");
    /**
     * @dev pass the request, along with all data, to Holograph Operator, to handle the cross-chain messaging logic
     */
    _operator().send{value: msg.value}(
      gasLimit,
      gasPrice,
      toChain,
      msg.sender,
      _jobNonce(),
      holographableContract,
      returnedPayload
    );
  }

It calls the operator's send() function. There's not too much happening there. Just some fee logic after which it sends the request over to the LayerZeroModule contract:

function send(
    uint256 gasLimit,
    uint256 gasPrice,
    uint32 toChain,
    address msgSender,
    uint256 nonce,
    address holographableContract,
    bytes calldata bridgeOutPayload
  ) external payable {
    require(msg.sender == _bridge(), "HOLOGRAPH: bridge only call");
    CrossChainMessageInterface messagingModule = _messagingModule();
    uint256 hlgFee = messagingModule.getHlgFee(toChain, gasLimit, gasPrice);
    address hToken = _registry().getHToken(_holograph().getHolographChainId());
    require(hlgFee < msg.value, "HOLOGRAPH: not enough value");
    payable(hToken).transfer(hlgFee);
    bytes memory encodedData = abi.encodeWithSelector(
      HolographBridgeInterface.bridgeInRequest.selector,
      /**
       * @dev job nonce is an incremented value that is assigned to each bridge request to guarantee unique hashes
       */
      nonce,
      /**
       * @dev including the current holograph chain id (origin chain)
       */
      _holograph().getHolographChainId(),
      /**
       * @dev holographable contract have the same address across all chains, so our destination address will be the same
       */
      holographableContract,
      /**
       * @dev get the current chain's hToken for native gas token
       */
      hToken,
      /**
       * @dev recipient will be defined when operator picks up the job
       */
      address(0),
      /**
       * @dev value is set to zero for now
       */
      hlgFee,
      /**
       * @dev specify that function call should not revert
       */
      true,
      /**
       * @dev attach actual holographableContract function call
       */
      bridgeOutPayload
    );
    /**
     * @dev add gas variables to the back for later extraction
     */
    encodedData = abi.encodePacked(encodedData, gasLimit, gasPrice);
    /**
     * @dev Send the data to the current Holograph Messaging Module
     *      This will be changed to dynamically select which messaging module to use based on destination network
     */
    messagingModule.send{value: msg.value - hlgFee}(
      gasLimit,
      gasPrice,
      toChain,
      msgSender,
      msg.value - hlgFee,
      encodedData
    );
    /**
     * @dev for easy indexing, an event is emitted with the payload hash for status tracking
     */
    emit CrossChainMessageSent(keccak256(encodedData));
  }

The LayerZeroModule then sends the request to the LayerZero endpoint:

  function send(
    uint256, /* gasLimit*/
    uint256, /* gasPrice*/
    uint32 toChain,
    address msgSender,
    uint256 msgValue,
    bytes calldata crossChainPayload
  ) external payable {
    require(msg.sender == address(_operator()), "HOLOGRAPH: operator only call");
    LayerZeroOverrides lZEndpoint;
    assembly {
      lZEndpoint := sload(_lZEndpointSlot)
    }
    // need to recalculate the gas amounts for LZ to deliver message
    lZEndpoint.send{value: msgValue}(
      uint16(_interfaces().getChainId(ChainIdType.HOLOGRAPH, uint256(toChain), ChainIdType.LAYERZERO)),
      abi.encodePacked(address(this), address(this)),
      crossChainPayload,
      payable(msgSender),
      address(this),
      abi.encodePacked(uint16(1), uint256(_baseGas() + (crossChainPayload.length * _gasPerByte())))
    );
  }

In the payload, it specifies that the target contract for LayerZero should be LayerZeroModule located on the dst chain. They all share the same address. You can find the endpoint's interface here: https://layerzero.gitbook.io/docs/guides/master/how-to-send-a-message

LayerZero then calls the lzReceive() function. This is the place where the NonBlocking approach has to be implemented. But, LayerZeroModule just does some input checks and routes the request to the operator:

  function lzReceive(
    uint16, /* _srcChainId*/
    bytes calldata _srcAddress,
    uint64, /* _nonce*/
    bytes calldata _payload
  ) external payable {
    assembly {
      /**
       * @dev check if msg.sender is LayerZero Endpoint
       */
      switch eq(sload(_lZEndpointSlot), caller())
      case 0 {
        /**
         * @dev this is the assembly version of -> revert("HOLOGRAPH: LZ only endpoint");
         */
        mstore(0x80, 0x08c379a000000000000000000000000000000000000000000000000000000000)
        mstore(0xa0, 0x0000002000000000000000000000000000000000000000000000000000000000)
        mstore(0xc0, 0x0000001b484f4c4f47524150483a204c5a206f6e6c7920656e64706f696e7400)
        mstore(0xe0, 0x0000000000000000000000000000000000000000000000000000000000000000)
        revert(0x80, 0xc4)
      }
      let ptr := mload(0x40)
      calldatacopy(add(ptr, 0x0c), _srcAddress.offset, _srcAddress.length)
      /**
       * @dev check if LZ from address is same as address(this)
       */
      switch eq(mload(ptr), address())
      case 0 {
        /**
         * @dev this is the assembly version of -> revert("HOLOGRAPH: unauthorized sender");
         */
        mstore(0x80, 0x08c379a000000000000000000000000000000000000000000000000000000000)
        mstore(0xa0, 0x0000002000000000000000000000000000000000000000000000000000000000)
        mstore(0xc0, 0x0000001e484f4c4f47524150483a20756e617574686f72697a65642073656e64)
        mstore(0xe0, 0x6572000000000000000000000000000000000000000000000000000000000000)
        revert(0x80, 0xc4)
      }
    }
    /**
     * @dev if validation has passed, submit payload to Holograph Operator for converting into an operator job
     */
    _operator().crossChainMessage(_payload);
  }

The operator doesn't put the logic in a try/catch block nor does it implement logic to retry failed requests:

    require(msg.sender == address(_messagingModule()), "HOLOGRAPH: messaging only call");
    /**
     * @dev would be a good idea to check payload gas price here and if it is significantly lower than current amount
     *      to set zero address as operator to not lock-up an operator unnecessarily
     */
    unchecked {
      bytes32 jobHash = keccak256(bridgeInRequestPayload);
      /**
       * @dev load and increment operator temp storage in one call
       */
      ++_operatorTempStorageCounter;
      /**
       * @dev use job hash, job nonce, block number, and block timestamp for generating a random number
       */
      uint256 random = uint256(keccak256(abi.encodePacked(jobHash, _jobNonce(), block.number, block.timestamp)));
      /**
       * @dev divide by total number of pods, use modulus/remainder
       */
      uint256 pod = random % _operatorPods.length;
      /**
       * @dev identify the total number of available operators in pod
       */
      uint256 podSize = _operatorPods[pod].length;
      /**
       * @dev select a primary operator
       */
      uint256 operatorIndex = random % podSize;
      /**
       * @dev If operator index is 0, then it's open season! Anyone can execute this job. First come first serve
       *      pop operator to ensure that they cannot be selected for any other job until this one completes
       *      decrease pod size to accomodate popped operator
       */
      _operatorTempStorage[_operatorTempStorageCounter] = _operatorPods[pod][operatorIndex];
      _popOperator(pod, operatorIndex);
      if (podSize > 1) {
        podSize--;
      }
      _operatorJobs[jobHash] = uint256(
        ((pod + 1) << 248) |
          (uint256(_operatorTempStorageCounter) << 216) |
          (block.number << 176) |
          (_randomBlockHash(random, podSize, 1) << 160) |
          (_randomBlockHash(random, podSize, 2) << 144) |
          (_randomBlockHash(random, podSize, 3) << 128) |
          (_randomBlockHash(random, podSize, 4) << 112) |
          (_randomBlockHash(random, podSize, 5) << 96) |
          (block.timestamp << 16) |
          0
      ); // 80 next available bit position && so far 176 bits used with only 128 left
      /**
       * @dev emit event to signal to operators that a job has become available
       */
      emit AvailableOperatorJob(jobHash, bridgeInRequestPayload);
    }
  }

As you can see, anybody can create a message through the LayerZero endpoint from src to dst chain. The remaining piece is to create a message that will for sure fail on the dst chain so that you can block it indefinitely. The easiest solution is to provide no tokens for gas when sending the message. The amount of gas is specified by the user when calling HolographBridge.bridgeOutRequest(). You specify both the gasPrice and gasLimit. Using those parameters, the HolographOperator.send() function determines the fees. If you set gasLimit to 0 you can initiate a tx with 1 wei for gas.

It's a little difficult to provide a full PoC here because of the LayerZero dependency. I tried to make it as clear as possible what the process would look like.

Tools Used

Recommended Mitigation Steps

Implement LayerZero's non blocking approach, see https://layerzero.gitbook.io/docs/faq/messaging-properties#message-ordering & https://layerzero.gitbook.io/docs/guides/advanced/nonblockinglzapp

[0xA5DF]

I don't think this is valid, since all LayerZero does here is to assign the job (the actual execution of the beaming) to an operator to be executed later, and the assigning part can't be failed easily.
Might be a QA though (just in case the assigning fails for some reason).

[gzeoneth]

Setting gasLimit/gasPrice to very low can only block the channel temporarily since one can donate fund to unblock it.

[0xA5DF]

Setting gasLimit/gasPrice to very low can only block the channel temporarily

Warden is trying to fail HolographBridge.bridgeOutRequest() which isn't even called by LayerZero, all that LayerZero does here is to assign the job to an operator via HolographOperator.crossChainMessage(), the rest is done by the operator (also, failing the job with a low gas limit wouldn't do anything to the protocol/operator, since it's inside a try/catch block)

[gzeoneth]

#445 might cause lzreceive to revert due to insufficient gas

[ACC01ADE]

LayerZero functionality either 100% succeeds or 100% fails. It is non-blocking. We are not using their helper libraries which would have created a blocking event in certain situations. And yes LayerZero is not in any way responsible for any aspects of the protocols except to deliver some bytes from one chain to another.

[ACC01ADE]

So to clarify, if a LayerZero tx fails, it will not block the entire channel. Everything else will work as expected and other LayerZero tx's can continue to be sent.