RAinUsTa - Malicious P2P peer can cause amplified memory consumption

[github-actions]

RAinUsTa

medium

Malicious P2P peer can cause amplified memory consumption

Summary

OP node subscribes to the pending L2 block gossip pubsub and expects properly encoded snappy block payloads.
For each incoming message, it's payload is snappy decompressed (if possible) and the content is hashed to a unique message ID based on the content itself used as part of message de-duplication. Default message ID TTL is configured to be 40s.

For valid messages, this mechanism works great.
For a malicious peer intent on sending non-snappy encoded data, e.g. [4]byte gibberish, this will cause amplified memory consumption.

Vulnerability Detail

Let's focus on the code path where the incoming payload is NOT a valid snappy compressed frame.

dLen, err := snappy.DecodedLen(pmsg.Data)
if err == nil && dLen <= maxGossipSize {
  ... decompress a valid block, etc...
}
if data == nil {
  // this piece here is critical since when someone send us malicious payload (random [4]byte), the data get passed through directly
  data = pmsg.Data
}
h := sha256.New()
... hasher proceeds to hash data
// the message ID is shortened to save space, a lot of these may be gossiped.
return string(h.Sum(nil)[:20])

Note that if the incoming data is gibberish, such as being generated by a random [4]byte, that data will be passed directly through to the hasher, producing a 32byte Sha256 hash output, which then is truncated to a 20byte prefix as the message id.

Impact

The impact is memory amplification where someone can take a crafted 4 byte output, and end up amplifying it to eat 20 bytes of memory space, an amplification factor of 5x.

When the message get processed later in the block validation logic, the snappy decompression error will result in:

log.Warn("invalid snappy compression length data", "err", err, "peer", id)
return pubsub.ValidationReject

but at this point, the 20 bytes of message ID is already stored.

Code Snippet

Relevant code snippet is:
https://github.com/sherlock-audit/2023-01-optimism/blob/main/optimism/op-node/p2p/gossip.go#L97-L105

Note it's already acknowledged that lots of message ID will be stored in memory and an effort was made to reduce consumption to 20 bytes per msg:

// the message ID is shortened to save space, a lot of these may be gossiped.
return string(h.Sum(nil)[:20])

However it's still not enough protection as explained.

Tool used

Manual Review

Recommendation

With regard to malformed snappy payload data, it does not seem necessary to generate unique message IDs for them. Consider not hashing the invalid payload data and assign invalid messages the same ID.

If you must generate unique ID even for malformed messages, then consider using xxhash uint64 short hashes for invalid messages and not using CPU and memory expensive Sha256.

[rcstanciu]

Comment from Optimism

---

Description: Memory amplification

Reason: The memory amplification here is very small (20 bytes per message). We should ignore these gibberish payloads, but it would be hard to exploit this.