Loops in bridging process disrupt transactions, potentially leading to denial of service.

[c4-bot-9]

Lines of code

https://github.com/code-423n4/2024-03-taiko/blob/f58384f44dbf4c6535264a472322322705133b11/packages/protocol/contracts/signal/SignalService.sol#L83-L134 https://github.com/code-423n4/2024-03-taiko/blob/f58384f44dbf4c6535264a472322322705133b11/packages/protocol/contracts/signal/SignalService.sol#L104-L129

Vulnerability details

Description

proveSignalReceived function in the SignalService contract is responsible for verifying the receipt of signals across multiple hops in the bridging process.

• It takes the following inputs:
  • _chainId: The ID of the chain where the signal originates.
  • _app: The address of the application that sent the signal.
  • _signal: The signal being verified.
  • _proof: The proof data for verifying the signal receipt across multiple hops.

• Code snippet: https://github.com/code-423n4/2024-03-taiko/blob/f58384f44dbf4c6535264a472322322705133b11/packages/protocol/contracts/signal/SignalService.sol#L83-L134

   function proveSignalReceived(
       uint64 _chainId,
       address _app,
       bytes32 _signal,
       bytes calldata _proof
   )
       public
       virtual
       validSender(_app)
       nonZeroValue(_signal)
   {
       HopProof[] memory hopProofs = abi.decode(_proof, (HopProof[]));
       if (hopProofs.length == 0) revert SS_EMPTY_PROOF();
  
       uint64 chainId = _chainId;
       address app = _app;
       bytes32 signal = _signal;
       bytes32 value = _signal;
       address signalService = resolve(chainId, "signal_service", false);
  
       HopProof memory hop;
       for (uint256 i; i < hopProofs.length; ++i) {
           hop = hopProofs[i];
  
           bytes32 signalRoot = _verifyHopProof(chainId, app, signal, value, hop, signalService);
           bool isLastHop = i == hopProofs.length - 1;
  
           if (isLastHop) {
               if (hop.chainId != block.chainid) revert SS_INVALID_LAST_HOP_CHAINID();
               signalService = address(this);
           } else {
               if (hop.chainId == 0 || hop.chainId == block.chainid) {
                   revert SS_INVALID_MID_HOP_CHAINID();
               }
               signalService = resolve(hop.chainId, "signal_service", false);
           }
  
           bool isFullProof = hop.accountProof.length > 0;
  
           _cacheChainData(hop, chainId, hop.blockId, signalRoot, isFullProof, isLastHop);
  
           bytes32 kind = isFullProof ? LibSignals.STATE_ROOT : LibSignals.SIGNAL_ROOT;
           signal = signalForChainData(chainId, kind, hop.blockId);
           value = hop.rootHash;
           chainId = hop.chainId;
           app = signalService;
       }
  
       if (value == 0 || value != _loadSignalValue(address(this), signal)) {
           revert SS_SIGNAL_NOT_FOUND();
       }
   }

  Users can craft a specially designed hopProofs array that contains a loop, where one or more hops point back to a previous hop in the sequence, creating a circular dependency.

• This loop can cause the function to enter an infinite loop, consume excessive gas, or exhibit unexpected behavior, potentially leading to token theft or denial of service.

• The vulnerability is present in the following lines of code: https://github.com/code-423n4/2024-03-taiko/blob/f58384f44dbf4c6535264a472322322705133b11/packages/protocol/contracts/signal/SignalService.sol#L104-L129

   for (uint256 i; i < hopProofs.length; ++i) {
       hop = hopProofs[i];
  
       // ...
  
       chainId = hop.chainId;
       app = signalService;
   }

  The proveSignalReceived function should verify the receipt of signals across multiple hops in the bridging process, ensuring a linear and non-circular sequence of hops.

• Each hop should represent a valid and unique step in the bridging sequence, with the chainId and app values updating accordingly.
• The function should successfully verify the signal receipt and complete the bridging process without any loops or unexpected behavior.

The problem is users can create a hopProofs array that contains a loop, where one or more hops point back to a previous hop, creating a circular dependency. When the function processes the malicious hopProofs array, it enters an infinite loop or exhibits unexpected behavior, such as:

• The loop can cause the function to consume all available gas, leading to a transaction failure and potentially disrupting the normal operation of the protocol.
• If the loop affects the token transfers or state updates, it can enable malicious actors to steal tokens by repeatedly executing the same hop and manipulating the token balances.
• The presence of loops can disrupt the normal functioning of the multi-hop bridging process, effectively causing a denial of service for legitimate users.

Impact

The presence of loops can disrupt the normal functioning of the multi-hop bridging process, preventing legitimate users from successfully completing their bridging transactions.

Tools Used

Manual Review

Recommended Mitigation Steps

function proveSignalReceived(
    uint64 _chainId,
    address _app,
    bytes32 _signal,
    bytes calldata _proof
)
    public
    virtual
    validSender(_app)
    nonZeroValue(_signal)
{
    HopProof[] memory hopProofs = abi.decode(_proof, (HopProof[]));
    if (hopProofs.length == 0) revert SS_EMPTY_PROOF();

    uint64 chainId = _chainId;
    address app = _app;
    bytes32 signal = _signal;
    bytes32 value = _signal;
    address signalService = resolve(chainId, "signal_service", false);

+   mapping(uint64 => mapping(address => bool)) visited;

    HopProof memory hop;
    for (uint256 i; i < hopProofs.length; ++i) {
        hop = hopProofs[i];

        // Loop detection
+       if (visited[hop.chainId][app]) revert SS_LOOP_DETECTED();
+       visited[hop.chainId][app] = true;

        // ...

        chainId = hop.chainId;
        app = signalService;
    }

    // ...
}

Assessed type

Loop

[c4-pre-sort]

minhquanym marked the issue as duplicate of #125

[c4-judge]

0xean marked the issue as satisfactory

[c4-judge]

0xean changed the severity to QA (Quality Assurance)

[c4-judge]

0xean marked the issue as grade-b