Null Termination Error in Attributes_Request Function of Attribute_Request.h in v0.15.0

[ahmetcobanoglu]

Description

In the function bool Attributes_Request(Attribute_Request_Callback<MaxAttributes> const & callback, char const * attribute_request_key, char const * attribute_response_key) within the Attribute_Request.h file, there is a potential null termination error due to inadequate buffer allocation for the request char array.

Impact

This issue can cause undefined behavior, including:

• Memory corruption.
• Invalid JSON serialization.
• Failure in sending correct attribute requests.

Environment

• Library version: v0.15.0
• Platform: ESP32S3
• Component: Attribute_Request.h

Code Reference

StaticJsonDocument<JSON_OBJECT_SIZE(1)> request_buffer;

// Calculate the size required for the char buffer containing all the attributes separated by a comma
size_t size = 0U;
for (const auto & att : attributes) {
    if (Helper::stringIsNullorEmpty(att)) {
        continue;
    }

    size += strlen(att);
    size += strlen(",");
}

// Initialize complete array to 0
char request[size] = {};
for (const auto & att : attributes) {
    if (Helper::stringIsNullorEmpty(att)) {
#if THINGSBOARD_ENABLE_DEBUG
        Logger::printfln(ATT_KEY_IS_NULL);
#endif // THINGSBOARD_ENABLE_DEBUG
        continue;
    }

    strncat(request, att, size);
    size -= strlen(att);
    strncat(request, ",", size);
    size -= strlen(",");
}

// Ensure to cast to const for ArduinoJson
request_buffer[attribute_request_key] = static_cast<const char*>(request);

Problem

The request buffer is allocated with a size exactly equal to the sum of the lengths of all attribute strings and commas. However, this calculation does not account for the null terminator required at the end of the string. This can lead to the following issues, particularly on the ESP32S3 platform, due to memory alignment:

1. Null Terminator Overwrite:

The request buffer is passed to the request_buffer[attribute_request_key] in the StaticJsonDocument. ArduinoJson attempts to reuse memory efficiently and may place additional data immediately after the allocated buffer. Without space for the null terminator, the next memory location is incorrectly interpreted, leading to undefined behavior.

2. Alignment-Specific Failures:

On ESP32S3, memory alignment quirks make the problem more pronounced when the buffer size matches 16x (e.g., 16, 32, 48, etc.). In these cases, the null terminator may fall into the adjacent memory block used by ArduinoJson, causing the JSON object to lose integrity.

Example Consider the following attribute:

std::aray<const char*,1> attribute = {"timeoutDuration"};

size = strlen("timeoutDuration") + strlen(",");
size = 15 + 1 = 16

When this buffer is passed to request_buffer[attribute_request_key], ArduinoJson might allocate memory immediately after the request buffer. Since the null terminator is missing, the next memory block is read incorrectly, leading to:

• Corrupted JSON data.
• Runtime errors or crashes.

Steps to Reproduce

1. Use attributes whose combined size (attribute + comma) aligns with 16x.
2. Observe memory corruption when the buffer is passed to request_buffer.

Proposed Fix

Increase the buffer size by 1 to accommodate the null terminator. Update the size calculation as follows:

size_t size = 0U;
for (const auto & att : attributes) {
    if (Helper::stringIsNullorEmpty(att)) {
        continue;
    }

    size += strlen(att);
    size += strlen(",");
}

// Add space for null terminator
size += 1;

char request[size] = {};

[BatuhanKaratas]

Upgrading the library to ArduinoJson v7 would help eliminate these memory alignment and buffer-sizing problems. Version 7 introduces more flexible memory management (dynamic/stream-based parsing) and a simpler API. This means:

No More Strict Buffer Size Calculations – We don’t have to worry about precise array sizes or missing null terminators. Safer Parsing and Fewer Errors – Dynamic memory usage reduces the risk of overwriting adjacent memory. Better Performance and Reliability – Improved parsing efficiency and easier handling of large or complex JSON documents.

For more details, you can see the official upgrade guide for ArduinoJson v7.

[MathewHDYT]

@BatuhanKaratas ArduinoJson 7 will not be supported, see this issue for more information. Nearly all ArduinoJson usage is abstraced by the library, nearly all ArduinoJson usage uses very small Documents, where the size is always known beforehand. Because the API has a fixed amount of expected parameters, therefore forcing to always use dynamic memory is not a good idea especially for low end boards taht are also supported by this library (Atmel AVR ATmega2560).

[BatuhanKaratas]

If the library is to continue supporting low-end boards, staying with v6 seems reasonable. What kind of solution can we develop regarding the issue mentioned above?

[MathewHDYT]

If the library is to continue supporting low-end boards, staying with v6 seems reasonable. What kind of solution can we develop regarding the issue mentioned above?

Simply the small proposed Fix in the code above, the buffer simply requires one additional char for the null character and the issue is resolved. I will integrate it in the library once I have time.

[samson0v]

@BatuhanKaratas @ahmetcobanoglu

:loudspeaker: A little announcement: We just launched our official Discord server, we'd love to see you there!