Skip to content

Commit 9d8e7bc

Browse files
bratpiorkabratpiorka
committed
fix potential mem corruption in utils_log_internal
1 parent c7fdae5 commit 9d8e7bc

2 files changed

Lines changed: 113 additions & 29 deletions

File tree

‎src/utils/utils_log.c‎

Lines changed: 80 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/*
22
*
3-
* Copyright (C) 2024-2025 Intel Corporation
3+
* Copyright (C) 2024-2026 Intel Corporation
44
*
55
* Under the Apache License v2.0 with LLVM Exceptions. See LICENSE.TXT.
66
* SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
@@ -72,6 +72,54 @@ typedef struct {
7272
utils_log_config_t loggerConfig = {false, false, LOG_ERROR,
7373
LOG_ERROR, NULL, ""};
7474

75+
static void log_buffer_advance(char **pos, size_t *size, size_t written) {
76+
ASSERT(pos);
77+
ASSERT(size);
78+
79+
if (*size <= 0) {
80+
return;
81+
}
82+
83+
if (written == 0) {
84+
*size = 0;
85+
return;
86+
}
87+
88+
// advance the buffer position and decrease the remaining size
89+
// take into the actual number of characters written
90+
if (written >= *size) {
91+
*pos += *size - 1;
92+
*size = 0;
93+
return;
94+
}
95+
96+
*pos += written;
97+
*size -= written;
98+
}
99+
100+
static void log_buffer_append(char **pos, size_t *size, const char *src) {
101+
ASSERT(pos);
102+
ASSERT(size);
103+
ASSERT(src);
104+
105+
if (*size <= 0) {
106+
return;
107+
}
108+
109+
size_t src_len = strlen(src);
110+
size_t copy_len = src_len;
111+
112+
if (copy_len >= *size) {
113+
copy_len = *size - 1;
114+
}
115+
memcpy(*pos, src, copy_len);
116+
117+
// ensure null-termination
118+
(*pos)[copy_len] = '\0';
119+
120+
log_buffer_advance(pos, size, src_len);
121+
}
122+
75123
static const char *level_to_str(utils_log_level_t l) {
76124
switch (l) {
77125
case LOG_DEBUG:
@@ -113,7 +161,7 @@ static void utils_log_internal(utils_log_level_t level, int perror,
113161

114162
char buffer[LOG_MAX];
115163
char *b_pos = buffer;
116-
int b_size = sizeof(buffer);
164+
size_t b_size = sizeof(buffer);
117165

118166
int tmp = 0;
119167
if (fileline == NULL) {
@@ -122,15 +170,21 @@ static void utils_log_internal(utils_log_level_t level, int perror,
122170
tmp = snprintf(b_pos, b_size, "%s %s: ", fileline, func);
123171
}
124172
ASSERT(tmp > 0);
173+
if (tmp < 0) {
174+
return; // snprintf error
175+
}
125176

126-
b_pos += (int)tmp;
127-
b_size -= (int)tmp;
177+
size_t written = (size_t)tmp;
178+
log_buffer_advance(&b_pos, &b_size, written);
128179

129180
tmp = vsnprintf(b_pos, b_size, format, args);
130181
ASSERT(tmp > 0);
182+
if (tmp < 0) {
183+
return; // vsnprintf error
184+
}
131185

132-
b_pos += (int)tmp;
133-
b_size -= (int)tmp;
186+
written = (size_t)tmp;
187+
log_buffer_advance(&b_pos, &b_size, written);
134188

135189
const char *postfix = "";
136190

@@ -139,10 +193,12 @@ static void utils_log_internal(utils_log_level_t level, int perror,
139193
strncat(b_pos, ": ", b_size);
140194
b_pos += 2;
141195
b_size -= 2;
196+
const char *err = "";
142197
#if defined(_WIN32)
143-
char err[80]; // max size according to msdn
144-
if (strerror_s(err, sizeof(err), errno)) {
145-
*err = '\0';
198+
char err_buff[80]; // max size according to msdn
199+
err = err_buff;
200+
if (strerror_s(err_buff, sizeof(err_buff), errno)) {
201+
*err_buff = '\0';
146202
postfix = "[strerror_s failed]";
147203
}
148204
#else
@@ -151,33 +207,27 @@ static void utils_log_internal(utils_log_level_t level, int perror,
151207

152208
#if defined(__APPLE__) || \
153209
((_POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600) && !_GNU_SOURCE)
154-
char err[1024];
155-
int ret = strerror_r(saveno, err, sizeof(err));
210+
char err_buff[1024];
211+
err = err_buff;
212+
int ret = strerror_r(saveno, err_buff, sizeof(err_buff));
156213
if (ret) {
157-
*err = '\0';
214+
*err_buff = '\0';
158215
postfix = "[strerror_r failed]";
159216
}
160217
if (errno == ERANGE) {
161218
postfix = "[truncated...]";
162219
}
163220
#else
164221
char err_buff[1024];
165-
const char *err = strerror_r(saveno, err_buff, sizeof(err_buff));
222+
err = strerror_r(saveno, err_buff, sizeof(err_buff));
166223
if (errno == ERANGE) {
167224
postfix = "[truncated...]";
168225
}
169226
#endif
170227

171228
errno = saveno;
172229
#endif
173-
strncpy(b_pos, err, b_size);
174-
size_t err_size = strlen(err);
175-
b_pos += err_size;
176-
b_size -= (int)err_size;
177-
if (b_size <= 0) {
178-
buffer[LOG_MAX - 1] =
179-
'\0'; //strncpy do not add \0 in case of overflow
180-
}
230+
log_buffer_append(&b_pos, &b_size, err);
181231
} else {
182232
postfix = "[truncated...]";
183233
}
@@ -190,7 +240,7 @@ static void utils_log_internal(utils_log_level_t level, int perror,
190240

191241
char header[LOG_HEADER];
192242
char *h_pos = header;
193-
int h_size = sizeof(header);
243+
size_t h_size = sizeof(header);
194244
memset(header, 0, sizeof(header));
195245

196246
if (loggerConfig.enableTimestamp) {
@@ -202,18 +252,19 @@ static void utils_log_internal(utils_log_level_t level, int perror,
202252
localtime_r(&now, &tm_info);
203253
#endif
204254

205-
ASSERT(h_size > 0);
206-
tmp = (int)strftime(h_pos, h_size, "%Y-%m-%dT%H:%M:%S ", &tm_info);
207-
h_pos += tmp;
208-
h_size -= tmp;
255+
written = strftime(h_pos, h_size, "%Y-%m-%dT%H:%M:%S ", &tm_info);
256+
log_buffer_advance(&h_pos, &h_size, written);
209257
}
210258

211259
if (loggerConfig.enablePid) {
212260
ASSERT(h_size > 0);
213261
tmp = snprintf(h_pos, h_size, "PID:%-6lu TID:%-6lu ",
214262
(unsigned long)pid, (unsigned long)tid);
215-
h_pos += tmp;
216-
h_size -= tmp;
263+
if (tmp < 0) {
264+
return; // snprintf error
265+
}
266+
written = (size_t)tmp;
267+
log_buffer_advance(&h_pos, &h_size, written);
217268
}
218269

219270
// We take twice header size here to ensure that
@@ -222,6 +273,7 @@ static void utils_log_internal(utils_log_level_t level, int perror,
222273
char logLine[LOG_MAX + LOG_HEADER * 2];
223274
snprintf(logLine, sizeof(logLine), "[%s%-5s UMF] %s%s\n", header,
224275
level_to_str(level), buffer, postfix);
276+
225277
FILE *out = loggerConfig.output ? loggerConfig.output : stderr;
226278
fputs(logLine, out);
227279

‎test/utils/utils_log.cpp‎

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (C) 2024-2025 Intel Corporation
1+
// Copyright (C) 2024-2026 Intel Corporation
22
// Under the Apache License v2.0 with LLVM Exceptions. See LICENSE.TXT.
33
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
44

@@ -368,6 +368,38 @@ TEST_F(test, long_log) {
368368
std::string(8190 - MOCK_FN_NAME.size(), 'x').c_str());
369369
}
370370

371+
TEST_F(test, long_func_name) {
372+
expect_fput_count = 1;
373+
expect_fflush_count = 1;
374+
loggerConfig =
375+
utils_log_config_t{false, false, LOG_DEBUG, LOG_DEBUG, stderr, ""};
376+
377+
std::string long_func(LOG_MAX, 'f');
378+
expected_message =
379+
"[DEBUG UMF] " + long_func.substr(0, LOG_MAX - 1) + "[truncated...]\n";
380+
381+
helper_test_log(LOG_DEBUG, long_func.c_str(), "%s", "tail");
382+
}
383+
384+
TEST_F(test, long_fileline_and_func_name) {
385+
expect_fput_count = 1;
386+
expect_fflush_count = 1;
387+
loggerConfig =
388+
utils_log_config_t{false, false, LOG_DEBUG, LOG_DEBUG, stderr, ""};
389+
390+
std::string long_fileline(LOG_MAX, 'l');
391+
std::string long_func(LOG_MAX, 'f');
392+
expected_message = "[DEBUG UMF] " + long_fileline.substr(0, LOG_MAX - 1) +
393+
"[truncated...]\n";
394+
395+
fput_count = 0;
396+
fflush_count = 0;
397+
utils_log(LOG_DEBUG, long_fileline.c_str(), long_func.c_str(), "%s",
398+
"tail");
399+
EXPECT_EQ(fput_count, expect_fput_count);
400+
EXPECT_EQ(fflush_count, expect_fflush_count);
401+
}
402+
371403
TEST_F(test, timestamp_log) {
372404
expect_fput_count = 1;
373405
expect_fflush_count = 1;

0 commit comments

Comments
 (0)