obront - Optimism node is susceptible to Gossip-related attacks due to a bug in handling its configuration

[sherlock-admin]

obront

medium

Optimism node is susceptible to Gossip-related attacks due to a bug in handling its configuration

Summary

A simple misordering of statements causes the user-defined options not be applied in the Gossip node.

Vulnerability Detail

The Optimism node incorporates a Gossip-client. It builds all the specific configurations in function NewGossipSub of gossip.go:

func NewGossipSub(p2pCtx context.Context, h host.Host, g ConnectionGater, cfg *rollup.Config, gossipConf GossipSetupConfigurables, m GossipMetricer, log log.Logger) (*pubsub.PubSub, error) {
    denyList, err := pubsub.NewTimeCachedBlacklist(30 * time.Second)
    if err != nil {
        return nil, err
    }
    params := BuildGlobalGossipParams(cfg)
    gossipOpts := []pubsub.Option{
        pubsub.WithMaxMessageSize(maxGossipSize),
        pubsub.WithMessageIdFn(BuildMsgIdFn(cfg)),
        pubsub.WithNoAuthor(),
        pubsub.WithMessageSignaturePolicy(pubsub.StrictNoSign),
        pubsub.WithSubscriptionFilter(BuildSubscriptionFilter(cfg)),
        pubsub.WithValidateQueueSize(maxValidateQueue),
        pubsub.WithPeerOutboundQueueSize(maxOutboundQueue),
        pubsub.WithValidateThrottle(globalValidateThrottle),
        pubsub.WithSeenMessagesTTL(seenMessagesTTL),
        pubsub.WithPeerExchange(false),
        pubsub.WithBlacklist(denyList),
        pubsub.WithGossipSubParams(params),
        pubsub.WithEventTracer(&gossipTracer{m: m}),
    }
    gossipOpts = append(gossipOpts, ConfigurePeerScoring(h, g, gossipConf, m, log)...)
    gossipOpts = append(gossipOpts, gossipConf.ConfigureGossip(&params)...)
    return pubsub.NewGossipSub(p2pCtx, h, gossipOpts...)
}

Firstly, fixed options are pushed to the array, including WithGossipSubParams(params). Later, ConfigureGossip(&params) is also appended.

gossipOpts = append(gossipOpts, gossipConf.ConfigureGossip(&params)...)

The issue is that the generated options completely ignore all the changes taking place in ConfigureGossip:

func (p *Config) ConfigureGossip(params *pubsub.GossipSubParams) []pubsub.Option {
    params.D = p.MeshD
    params.Dlo = p.MeshDLo
    params.Dhi = p.MeshDHi
    params.Dlazy = p.MeshDLazy

    // in the future we may add more advanced options like scoring and PX / direct-mesh / episub
    return []pubsub.Option{
        pubsub.WithFloodPublish(p.FloodPublish),
    }
}

The params are updated from the user specified gossipConf, but recall that they are already stored in the option array. The only user specified option actually used is FloodPublish which is returned from the function.

Impact

User specified gossip values are ignored, which impairs node functionality. It may be important to set the D values such as Dlazy higher when the node is being suffocated and needs to bypass censorship.

Code Snippet

https://github.com/ethereum-optimism/optimism/blob/9b9f78c6613c6ee53b93ca43c71bb74479f4b975/op-node/p2p/gossip.go#L177

Tool used

Manual Review

Recommendation

Re-order the statements, so that the params filled in by ConfigureGossip will be sent to gossipOpts.

[hrishibhat]

Sponsor comment: ConfigureGossip mutates the mesh params, after a copy of the parameters has already been made by WithGossipSubParams, causing the D, Dlo, Dhi, Dlazy CLI parameters to not be applied as expected.

[GalloDaSballo]

I believe the node can be restarted with proper params, so agree with Low Severity

[zobront]

Escalate for 10 USDC

Judge's comment: "I believe the node can be restarted with proper params, so agree with Low Severity" is incorrect.

As stated, the user-specified parameters are ignored by the implementation as they are already set in stone to the default values. Moreover, users cannot be expected to be aware of this bug, so the unintended properties will continue to be used, leading to detrimental performance.

Unconfigurability of gossip parameters can lead to censorship-related and consensus-related issues, as described in the submission.

We've seen plenty of non-principal and temporary bugs being awarded Medium severity in this contest and believe this to be the appropriate severity.

[sherlock-admin]

Escalate for 10 USDC

Judge's comment: "I believe the node can be restarted with proper params, so agree with Low Severity" is incorrect.

As stated, the user-specified parameters are ignored by the implementation as they are already set in stone to the default values. Moreover, users cannot be expected to be aware of this bug, so the unintended properties will continue to be used, leading to detrimental performance.

Unconfigurability of gossip parameters can lead to censorship-related and consensus-related issues, as described in the submission.

We've seen plenty of non-principal and temporary bugs being awarded Medium severity in this contest and believe this to be the appropriate severity.

You've created a valid escalation for 10 USDC!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[maurelian]

The full comment we'd intended to post here was this:

ConfigureGossip mutates the mesh params, after a copy of the parameters has already been made by WithGossipSubParams, causing the D, Dlo, Dhi, Dlazy CLI parameters to not be applied as expected.

Code:
- Loaded here: https://github.com/ethereum-optimism/optimism/blob/0d7859bbfcc3b8d5c9259f3b5baf2a4b6e7dc7e4/op-node/p2p/cli/load_config.go#L357
- Applied here: https://github.com/ethereum-optimism/optimism/blob/8fd1bb42b4a945ed13d10e14dbcf72e33c070de8/op-node/p2p/gossip.go#L128
- Accidentally copied before application here: https://github.com/ethereum-optimism/optimism/blob/8fd1bb42b4a945ed13d10e14dbcf72e33c070de8/op-node/p2p/gossip.go#L173
- Defaults still applied here: https://github.com/ethereum-optimism/optimism/blob/8fd1bb42b4a945ed13d10e14dbcf72e33c070de8/op-node/p2p/gossip.go#L141
- CLI parameters hidden here: https://github.com/ethereum-optimism/optimism/blob/0d7859bbfcc3b8d5c9259f3b5baf2a4b6e7dc7e4/op-node/flags/p2p_flags.go#L240-L271

The CLI parameters are hidden: this mechanism is for advanced optimization, not something we recommend users to change. Some ethereum L1 clients who use GossipSub for example do not make this configurable by users at all.

Impact is strictly limited to advanced users who tweak the hidden mesh settings to be different from recommended defaults. Impact is thus not as severe as claimed in the issue, but the issue itself is valid.

[hrishibhat]

Escalation rejected

After further discussion and Considering the severity of this issue as low based on the Sponsor comment.

[sherlock-admin]

Escalation rejected

After further discussion and Considering the severity of this issue as low based on the Sponsor comment.

This issue's escalations have been rejected!

Watsons who escalated this issue will have their escalation amount deducted from their next payout.