diff --git a/tests/testDriver_logger_nul_termination.c b/tests/testDriver_logger_nul_termination.c new file mode 100644 index 0000000..71f9211 --- /dev/null +++ b/tests/testDriver_logger_nul_termination.c @@ -0,0 +1,65 @@ +// SPDX-License-Identifier: Apache-2.0 +/* + * Copyright IBM Corp. 2024 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "testsupport.h" + +#include + +/* + * Regression coverage for log_message()'s no-formatting path. + * + * Historical behavior (bug): + * - When arg == NULL, log_message() used strncpy() without guaranteeing + * NUL-termination and then called strlen() on the buffer. + * - If the message exactly filled the remaining buffer, strlen() could read + * past the end of msg_buf (undefined behavior). + * + * Expected behavior (fixed): + * - The message is always NUL-terminated, even when truncated. + */ + +void setUp(void) {} +void tearDown(void) {} + +void test_log_message_arg_null_is_nul_terminated(void) { + log_level = LOGLEVEL_ERROR; + log_module[0] = '\0'; + + // Construct a message that is far larger than the logger's internal buffer. + // We intentionally omit a trailing newline to exercise the auto-newline path. + char long_msg[2048]; + memset(long_msg, 'B', sizeof(long_msg) - 1); + long_msg[sizeof(long_msg) - 1] = '\0'; + + char buf_stderr[BUFSIZ] = {0}; + + stderr_to_pipe(); + log_message(LOGLEVEL_ERROR, __func__, __FILE__, __LINE__, long_msg, NULL); + restore_stderr(buf_stderr, BUFSIZ); + + // If we got here, we did not crash. Also ensure some of our payload made it. + TEST_ASSERT_NOT_NULL_MESSAGE(strstr(buf_stderr, "BBB"), + "Expected payload not found in stderr"); +} + +int main(void) { + UNITY_BEGIN(); + + RUN_TEST(test_log_message_arg_null_is_nul_terminated); + + return UNITY_END(); +} diff --git a/zdnn/logger.c b/zdnn/logger.c index f01df50..e30352a 100644 --- a/zdnn/logger.c +++ b/zdnn/logger.c @@ -130,13 +130,28 @@ void log_message(log_levels lvl, const char *func_name, const char *file_name, snprintf(msg_buf, LOG_BUFFER_LEN, LOG_HEADER, log_levels_str[lvl_real], func_name, file_name, line_no); + // snprintf/vsnprintf return the number of chars that *would* have been + // written (excluding the terminator). Clamp to keep all following pointer + // arithmetic and length calculations in-bounds even when the header is + // truncated. + if (header_len < 0) { + header_len = 0; + } else if (header_len >= LOG_BUFFER_LEN) { + header_len = LOG_BUFFER_LEN - 1; + } + + size_t remaining = LOG_BUFFER_LEN - (size_t)header_len; + if (arg) { - vsnprintf(msg_buf + header_len, LOG_BUFFER_LEN - header_len, format, arg); + (void)vsnprintf(msg_buf + header_len, remaining, format, arg); } else { - // nothing to format, copy it as-is - strncpy(msg_buf + header_len, format, LOG_BUFFER_LEN - header_len); + // Nothing to format, copy it as-is but ensure NUL termination. + (void)snprintf(msg_buf + header_len, remaining, "%s", format); } + // Defensively ensure termination even if remaining == 0. + msg_buf[LOG_BUFFER_LEN - 1] = '\0'; + if (lvl_real == LOGLEVEL_ERROR || lvl_real == LOGLEVEL_FATAL) { stream = stderr; } else {