dlt-user: fix crash with certain strings #463
Merged
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.
make sure that a string with exactly UINT16_MAX bytes does not overflow the dlt buffer
Given the example below there is a crash is libdlt
The expansion of the macro looks like this
The crash now happens due to the following chain of events.
dlt_user_log_write_start(&mycontext, &log_local, DLT_LOG_INFO);
is callingdlt_user_log_write_string_utils_attr
Which is casting the string length into
uint16_t
. This is fine as our string is exactly as long asuint16_t
allows.Now
dlt_user_log_write_sized_string_utils_attr
is called.This sets
uint16_t arg_size = (uint16_t) (length + 1)
. Which will overflow the uint16_t andarg_size
will be0
.The code above also won't detect that the string is too large as the uin16_t also overflows and the sum will be much smaller as
dlt_user.log_buf_len
resulting in the truncation check to eval to false.Now
log->size
is incremented in several places.In my example with enabled verbose mode it's
6
The code above is now copying into the buffer at
buffer + log->size
.As the original length with 65535 is used we're writing 6 bytes behind the actual buffer.
With larger strings, their length is still a multiple of
uint16_t max
, the same crash occurred.If it's not a multiple the application does not crash but reads a certain amount of bytes from the buffer, truncating the log without any message that it was cut of.
It's fixed by using size_t for the calculation, so the truncation is working properly.
After the fix
case DLT_RETURN_USER_BUFFER_FULL:
is used and the truncated string will be logged.The program was tested solely for our own use cases, which might differ from yours.
Licensed under Mozilla Public License Version 2.0
Alexander Mohr, [email protected], Mercedes-Benz Tech Innovation GmbH, imprint