Skip to content
This repository has been archived by the owner on May 26, 2023. It is now read-only.

obront - Client will accept invalid blocks from gossip channels due to insufficient L1BlockInfo decoding #282

Closed
github-actions bot opened this issue Feb 20, 2023 · 1 comment
Labels
Non-Reward This issue will not receive a payout

Comments

@github-actions
Copy link

obront

low

Client will accept invalid blocks from gossip channels due to insufficient L1BlockInfo decoding

Summary

The Optimism derivation driver is responsible for deducing the safe L2 state at any time. Clients hear about new L2 blocks using gossip. These blocks are decoded using various functions, specifically the first TX which is always the L1BlockInfo is decoded by l1_block_info.go's UnmarshalBinary. However, the decoding of this block skips the 4byte check required to be sure of its validity.

Vulnerability Detail

We can see that L1BlockInfo decoding skips the first 4 bytes check:

if len(data) != L1InfoLen {
	return fmt.Errorf("data is unexpected length: %d", len(data))
}
var padding [24]byte
offset := 4
info.Number = binary.BigEndian.Uint64(data[offset+24 : offset+32])
if !bytes.Equal(data[offset:offset+24], padding[:]) {
	return fmt.Errorf("l1 info number exceeds uint64 bounds: %x", data[offset:offset+32])
}

Therefore, invalid gossip messages will be accepted which may cause degraded performance. When the block is pulled from L1, the pipeline will insert correct L1BlockInfo and ditch the gossip originated message. However, there still will be degraded performance.

It may or may not be possible to trigger a derivation reset using this malicious block injection technique.

Impact

Client will accept invalid blocks from gossip channels.

Code Snippet

https://github.com/ethereum-optimism/optimism/blob/407f97b9d13448b766624995ec824d3059d4d4f6/op-node/rollup/derive/l1_block_info.go#L71

Tool used

Manual Review

Recommendation

Validate that the first 4 bytes of an L1InfoBlock contain the correct method hash.

@rcstanciu
Copy link
Contributor

Comment from Optimism


Description: L1Info message hash isn't checked

Reason: We should check this as part of block validation.

#Action: Add a check for the function selector

@sherlock-admin sherlock-admin added the Non-Reward This issue will not receive a payout label Feb 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Non-Reward This issue will not receive a payout
Projects
None yet
Development

No branches or pull requests

2 participants