Harden beats packet parsing#26139
Conversation
| private static final int MAX_JSON_FRAME_BYTES = 16 * 1024 * 1024; | ||
| private static final int MAX_DATA_PAIRS = 1024; | ||
| private static final int MAX_DATA_ITEM_BYTES = 1024 * 1024; |
There was a problem hiding this comment.
What's the reasoning behind the chosen values?
I'm wondering if we could use a single, generous, MAX_EVENT_SIZE setting instead so that we don't have to impose additional restrictions on the structure of data frames.
For JSON frames it would be a drop in replacement for MAX_JSON_FRAME_BYTES, so we could reject straight away like below. For data frames we could track read bytes and abort after MAX_EVENT_SIZE was read.
There was a problem hiding this comment.
I've chosen the values to be large enough to never interfere with normal operations but small enough to not allow malicious packets (that simple advertise a wrong, large size) to allocated enormous amounts of memory.
data_pairs and data_item_bytes are, afaik, the legacy v1 protocol and really shouldn't appear anymore. The JSON one is a single message: while 16MB is probably stupidly large, I think it's still small enough to live with the overhead.
This patch really is about guarding against malformed packets/sizes and exit early before we allocate.
2 participants
Description
When parsing beats packets we allocated memory based on the declared frame sizes for the three possible frame types.
As the value is untrusted, we now reject unreasonable large values before allocating memory.
Motivation and Context
Malformed packets could lead to unreasonable memory allocation during beats protocol parsing.
How Has This Been Tested?
Unit tests
Types of changes
Checklist: