A malicious user can become a permissioned relayer for IBC
Description
When a new bridge is created, a hook is run to register the challenger as the permissioned relayer for any configured IBC channels:
func (h BridgeHook) BridgeCreated(
ctx context.Context,
bridgeId uint64,
bridgeConfig ophosttypes.BridgeConfig,
) error {
hasPermChannels, metadata := hasPermChannels(bridgeConfig.Metadata)
if !hasPermChannels {
return nil
}
challenger, err := h.ac.StringToBytes(bridgeConfig.Challenger)
if err != nil {
return err
}
sdkCtx := sdk.UnwrapSDKContext(ctx)
for _, permChannel := range metadata.PermChannels {
portID, channelID := permChannel.PortID, permChannel.ChannelID
if seq, ok := h.IBCChannelKeeper.GetNextSequenceSend(sdkCtx, portID, channelID); !ok {
return channeltypes.ErrChannelNotFound.Wrap("failed to register permissioned relayer")
} else if seq != 1 {
return channeltypes.ErrChannelExists.Wrap("cannot register permissioned relayer for the channel in use")
}
// register challenger as channel relayer
if err = h.IBCPermKeeper.SetPermissionedRelayer(sdkCtx, portID, channelID, challenger); err != nil {
return err
}
}
return nil
}The IBC channel must not be in use, which is checked by ensuring that the sequence number for it is one.
The issue is that a malicious user can watch for newly created IBC channels, then create a new bridge to become the permissioned relayer for the channel.
When the real bridge is created, the correct challenger will become the channel relayer, but the malicious user can update their bridge to take it over again:
func (h BridgeHook) BridgeChallengerUpdated(
ctx context.Context,
bridgeId uint64,
bridgeConfig ophosttypes.BridgeConfig,
) error {
hasPermChannels, metadata := hasPermChannels(bridgeConfig.Metadata)
if !hasPermChannels {
return nil
}
challenger, err := h.ac.StringToBytes(bridgeConfig.Challenger)
if err != nil {
return err
}
sdkCtx := sdk.UnwrapSDKContext(ctx)
for _, permChannel := range metadata.PermChannels {
portID, channelID := permChannel.PortID, permChannel.ChannelID
// update relayer to a new challenger
if err = h.IBCPermKeeper.SetPermissionedRelayer(sdkCtx, portID, channelID, challenger); err != nil {
return err
}
}
return nil
}Impact
A malicious user can become the permissioned relayer for newly created IBC channels and continue to take over the relayer even after the real bridge is created.
Recommendations
If the permissioned relayer is already set, an error should be raised to indicate that someone else has already claimed the channel.
If possible, the IBC channel and bridge creation should happen within the same transaction to prevent anyone from being able to take over the channel.