Skip to content

Commit 1438534

Browse files
kpschoedelpull[bot]
authored andcommitted
Fix unbounded stack in emberAfPrintBuffer() (#4633)
* Fix unbounded stack in emberAfPrintBuffer() #### Problem emberAfPrintBuffer() allocates stack space based on an argument, with no upper bound. This could lead to stack exhaustion on small devices. #### Summary of Changes Print long buffers in multiple segments. Fixes #3662 - Unbounded stack in emberAfPrintBuffer() * Use CHIP_DEVICE_CONFIG_LOG_MESSAGE_MAX_SIZE for buffer size * TODO * casts
1 parent 73a7721 commit 1438534

File tree

1 file changed

+24
-18
lines changed

1 file changed

+24
-18
lines changed

src/app/util/ember-print.cpp

+24-18
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/**
22
*
3-
* Copyright (c) 2020 Project CHIP Authors
3+
* Copyright (c) 2020-2021 Project CHIP Authors
44
*
55
* Licensed under the Apache License, Version 2.0 (the "License");
66
* you may not use this file except in compliance with the License.
@@ -20,6 +20,8 @@
2020

2121
#include "debug-printing.h"
2222

23+
#include <platform/CHIPDeviceConfig.h>
24+
#include <support/CodeUtils.h>
2325
#include <support/logging/CHIPLogging.h>
2426

2527
bool emberAfPrintReceivedMessages = true;
@@ -28,7 +30,7 @@ using namespace chip::Logging;
2830

2931
void emberAfPrint(int category, const char * format, ...)
3032
{
31-
if (format != NULL)
33+
if (format != nullptr)
3234
{
3335
va_list args;
3436
va_start(args, format);
@@ -39,7 +41,7 @@ void emberAfPrint(int category, const char * format, ...)
3941

4042
void emberAfPrintln(int category, const char * format, ...)
4143
{
42-
if (format != NULL)
44+
if (format != nullptr)
4345
{
4446
va_list args;
4547
va_start(args, format);
@@ -48,35 +50,39 @@ void emberAfPrintln(int category, const char * format, ...)
4850
}
4951
}
5052

51-
// TODO: issue #3662 - Unbounded stack in emberAfPrintBuffer()
52-
#pragma GCC diagnostic push
53-
#pragma GCC diagnostic ignored "-Wstack-usage="
53+
// TODO: add unit tests.
5454

5555
void emberAfPrintBuffer(int category, const uint8_t * buffer, uint16_t length, bool withSpace)
5656
{
57-
if (buffer != NULL && length > 0)
57+
if (buffer != nullptr && length > 0)
5858
{
59-
const char * perByteFormatStr = withSpace ? "%02hhX " : "%02hhX";
60-
uint8_t perByteCharCount = withSpace ? 3 : 2;
59+
constexpr uint16_t kBufferSize = CHIP_DEVICE_CONFIG_LOG_MESSAGE_MAX_SIZE;
60+
const char * perByteFormatStr = withSpace ? "%02hhX " : "%02hhX";
61+
const uint8_t perByteCharCount = withSpace ? 3 : 2;
62+
const uint16_t bytesPerBuffer = static_cast<uint16_t>((kBufferSize - 1) / perByteCharCount);
63+
char result[kBufferSize];
6164

62-
uint32_t outStringLength = length * perByteCharCount + 1;
63-
char result[outStringLength];
64-
for (uint32_t dst_idx = 0, index = 0; dst_idx < outStringLength - 1 && index < length; dst_idx += perByteCharCount, index++)
65+
uint16_t index = 0;
66+
while (index < length)
6567
{
66-
67-
snprintf(result + dst_idx, outStringLength - dst_idx, perByteFormatStr, buffer[index]);
68+
const uint16_t remainingBytes = static_cast<uint16_t>(length - index);
69+
const uint16_t segmentLength = chip::min(bytesPerBuffer, remainingBytes);
70+
const uint16_t segmentEnd = static_cast<uint16_t>(index + segmentLength);
71+
const uint32_t outStringEnd = segmentLength * perByteCharCount;
72+
for (uint32_t dst_idx = 0; dst_idx < outStringEnd && index < segmentEnd; dst_idx += perByteCharCount, index++)
73+
{
74+
snprintf(result + dst_idx, outStringEnd - dst_idx + 1, perByteFormatStr, buffer[index]);
75+
}
76+
result[outStringEnd] = 0;
77+
emberAfPrint(category, "%s", result);
6878
}
69-
result[outStringLength - 1] = 0;
70-
emberAfPrint(category, "%s", result);
7179
}
7280
else
7381
{
7482
emberAfPrint(EMBER_AF_PRINT_CORE, "NULL");
7583
}
7684
}
7785

78-
#pragma GCC diagnostic pop // -Wstack-usage
79-
8086
void emberAfPrintString(int category, const uint8_t * string)
8187
{
8288
emberAfPrint(category, "%s", string);

0 commit comments

Comments
 (0)