Description

To include important parameters in signatures, the bridge packs them together and hashes them. This data is stored in the connectionId slot of the Agent struct, which has an associated function hash for creating the actual signed message.

struct Agent {
  string source;
  bytes32 connectionId;
}

In some functions, the hashed data in connectionId includes the name of an action:

Agent memory agent = Agent("a", keccak256(abi.encode("modifyLocker", locker, isLocker, nonce)));

However, the connectionIds used in the requestWithdrawal and updateValidatorSet Agent's do not. Instead, they rely on the arguments being different to prevent valid signatures from being used in the wrong function. From requestWithdrawal and updateValidatorSet:

Agent memory agent = Agent("a", keccak256(abi.encode(msg.sender, usdc, nonce)));
Agent memory agent = Agent(
  "a",
  keccak256(
    abi.encode(
      newValidatorSet.epoch,
      newValidatorSet.hotAddresses,
      newValidatorSet.coldAddresses,
      newValidatorSet.powers
    )
  )
);

Impact

This introduces some maintenance risk: updating these signature arguments may have the unintended consequence of allowing confusion between the two types. That might allow users to use withdrawal signatures to maliciously update validators.

Recommendations

We recommend consistently prefixing all messages with the action to guarantee that changes in arguments do not cause bugs.

Remediation

This issue has been acknowledged by Hyperliquid, and a fix was implemented in commit b198269c↗.