From 2347ceddc51c4d951cd4a297824cd2629b4bea5f Mon Sep 17 00:00:00 2001 From: Old-Ding <35417409+Old-Ding@users.noreply.github.com> Date: Sat, 11 Jul 2026 15:51:48 +0800 Subject: [PATCH] Fix bounded concatenation in char arrays rcutils_char_array_strncat copied exactly n bytes even when the source string ended earlier, reading and counting data past the terminator. Use the bounded source length for copying and reject destination length overflow. Signed-off-by: Old-Ding <35417409+Old-Ding@users.noreply.github.com> --- src/char_array.c | 11 +++++++++-- test/test_char_array.cpp | 4 +++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/char_array.c b/src/char_array.c index 4f69c4a7..c6d28bc1 100644 --- a/src/char_array.c +++ b/src/char_array.c @@ -13,7 +13,9 @@ // limitations under the License. #include +#include #include "rcutils/error_handling.h" +#include "rcutils/strnlen.h" #include "rcutils/types/char_array.h" #define MIN(a, b) ((a) < (b) ? (a) : (b)) @@ -220,14 +222,19 @@ rcutils_char_array_strncat(rcutils_char_array_t * char_array, const char * src, // The buffer length always contains the trailing \0, so the strlen is one less than that. current_strlen = char_array->buffer_length - 1; } - size_t new_length = current_strlen + n + 1; + size_t copy_length = rcutils_strnlen(src, n); + if (copy_length > SIZE_MAX - current_strlen - 1) { + RCUTILS_SET_ERROR_MSG("requested size for char_array too large"); + return RCUTILS_RET_BAD_ALLOC; + } + size_t new_length = current_strlen + copy_length + 1; rcutils_ret_t ret = rcutils_char_array_expand_as_needed(char_array, new_length); if (ret != RCUTILS_RET_OK) { // rcutils_char_array_expand_as_needed already set the error return ret; } - memcpy(char_array->buffer + current_strlen, src, n); + memcpy(char_array->buffer + current_strlen, src, copy_length); char_array->buffer[new_length - 1] = '\0'; char_array->buffer_length = new_length; diff --git a/test/test_char_array.cpp b/test/test_char_array.cpp index 657d2f7e..d7523fba 100644 --- a/test/test_char_array.cpp +++ b/test/test_char_array.cpp @@ -156,7 +156,9 @@ TEST_F(ArrayCharTest, strcat) { EXPECT_STREQ("1234", char_array.buffer); EXPECT_EQ(5lu, char_array.buffer_length); - EXPECT_EQ(RCUTILS_RET_OK, rcutils_char_array_strcat(&char_array, "56")); + const char source[] = {'5', '6', '\0', 'x', 'x'}; + EXPECT_EQ(RCUTILS_RET_OK, + rcutils_char_array_strncat(&char_array, source, sizeof(source))); EXPECT_STREQ("123456", char_array.buffer); EXPECT_EQ(7lu, char_array.buffer_length);