fix(kscrash): prevent stack overflow escaping strings to JSON #739
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Goal
In load testing concurrent
notify
calls, it was found that an occasional stack overflow is possible if a string being converted to JSON contains a significant number of control characters.The codec escapes control characters by adding "\u" followed by the hex value of the character. However this means that up to 6 characters can be added for each control character. Because a fixed-size buffer is used to work on, this can cause a stack overflow if it contains enough characters.
Design
To account for whitespace replacements (e.g. tabs to "\t") we were previously only allowing the string to be half the size of the buffer. I believe this pre-dated the inclusion of the "\uXXXX" logic and wasn't updated at the time. However to only allow 1/6 of the buffer seems too small, so I've removed this bit of logic from
bsg_ksjsoncodec_i_addEscapedString
and added an extra call toaddJSONData
insidebsg_ksjsoncodec_i_appendEscapedString
so we add the existing string and reset the buffer if it's possible to exceed it on the next character.Tests
Added some unit tests to ensure the logic works for longer strings.