Avoid parsing large sizes for messages #1441
Conversation
| l := len(d) | ||
| if l == 0 { | ||
| if l == 0 || l > maxParseSizeLen { |
There was a problem hiding this comment.
Although I agree that NATS users should not send large messages, you are limiting it to about 99MB. Not sure if this is going to break users that have bumped the max payload.
I think that the new issue reported is simply that the server was trying to allocate more memory that the machine had, not that there was a rollover to negative that was not handled properly in the past.
There was a problem hiding this comment.
This is the original fix: #1053
There was a problem hiding this comment.
We have max pending buffers internal that are 64M so I thought that was reasonable.
Signed-off-by: Derek Collison <derek@nats.io>
There was a problem hiding this comment.
LGTM. As discussed, servers that are deployed and have a max_payload set (by default it is 1MB) are not impacted by this PUB foo 333333333333333333\r\n protocol. If no limit is set (or set to very high value), then this change will apply a hard limit of a size of 9 digits. It is still possible that the server will produce the stack because it would fail to allocate memory (the make([]byte) would panic due to lack of memory). Ultimately, we should warn people who set a big/unlimited max_payload and in the future even error if the value is more than a certain size (which one is tbd).
Fixes https://www.twistlock.com/labs-blog/finding-dos-vulnerability-nats-go-fuzz-cve-2019-13126/
Signed-off-by: Derek Collison derek@nats.io
/cc @nats-io/core