code-423n4 / 2024-05-olas-findings

13 stars 4 forks source link

Wormhole uses Dispenser contract address as a refund address on l2 chain, resulting in a loss of funds #91

Closed howlbot-integration[bot] closed 4 months ago

howlbot-integration[bot] commented 4 months ago

Lines of code

https://github.com/code-423n4/2024-05-olas/blob/main/tokenomics/contracts/staking/WormholeDepositProcessorL1.sol#L85

Vulnerability details

Impact

Wormhole has a refund mechanism that refunds on the destination chain instead of the source as the refundChain passed by default is L2, but WormholeDepositProcessorL1::_sendMessage passes msg.sender as a refunder, resulting in all the refunds being sent to the wrong recipient.

Proof of Concept

In case no explicit refundAccount has been provided to the bridgePayload msg.sender , which is the Dispenser contract, will be used by default. The problem is that since Dispenser is a smart contract, it won’t have the same address on the L2 chains where the refund will happen. This will result in all the ever-refunded tokens being sent to a random recipient on the L2 chain when the refund happens.

/// @inheritdoc DefaultDepositProcessorL1
  function _sendMessage(
      address[] memory targets,
      uint256[] memory stakingIncentives,
      bytes memory bridgePayload,
      uint256 transferAmount
  ) internal override returns (uint256 sequence) {
...MORE CODE
      // If refundAccount is zero, default to msg.sender
      if (refundAccount == address(0)) {
          refundAccount = msg.sender;
      }

      sequence = sendTokenWithPayloadToEvm(
          uint16(wormholeTargetChainId), 
          l2TargetDispenser, 
          data, 
          0,
          gasLimitMessage, 
          olas,//REVIEW this should be set to the address of Olas on the destination chain
          transferAmount, 
          uint16(l2TargetChainId), 
          refundAccount);//@audit wrong address
  }

Refunds in Wormhole are for the unused gas on the destination chain execution. We say that refunds will be happening often as the gasLimitMessage is an arbitrary parameter provided from the caller as well, although having better validation.

Tools Used

Manual Review

Recommended Mitigation Steps

Revert the transaction in case address(0) is passed as refundAddress to the bridgePayload.

Assessed type

Token-Transfer

c4-judge commented 4 months ago

0xA5DF marked the issue as satisfactory