From c30ce0b3d524a2c15bc688356d50d38fa9b43f85 Mon Sep 17 00:00:00 2001 From: Christian Babeux Date: Mon, 20 Aug 2012 15:56:06 -0400 Subject: [PATCH] Fix: Possible buffer overflows in strncat() usage When using strncat, the size_t n argument must indicate the left over space remaining in the buffer, *not* the total buffer size. Also, proper care must be taken for the case where src contains n or more bytes and thus allow space for the null terminating byte appended to dest (e.g. strncat() will write n+1 bytes). Signed-off-by: Christian Babeux Signed-off-by: David Goulet --- src/bin/lttng-sessiond/cmd.c | 12 ++++++++---- src/bin/lttng-sessiond/consumer.c | 7 +++++-- src/bin/lttng-sessiond/main.c | 3 ++- src/common/utils.c | 2 +- 4 files changed, 16 insertions(+), 8 deletions(-) diff --git a/src/bin/lttng-sessiond/cmd.c b/src/bin/lttng-sessiond/cmd.c index d36966322..1b1f87b8e 100644 --- a/src/bin/lttng-sessiond/cmd.c +++ b/src/bin/lttng-sessiond/cmd.c @@ -385,7 +385,8 @@ static int add_uri_to_consumer(struct consumer_output *consumer, if (uri->stype == LTTNG_STREAM_CONTROL) { /* On a new subdir, reappend the default trace dir. */ - strncat(consumer->subdir, default_trace_dir, sizeof(consumer->subdir)); + strncat(consumer->subdir, default_trace_dir, + sizeof(consumer->subdir) - strlen(consumer->subdir) - 1); DBG3("Append domain trace name to subdir %s", consumer->subdir); } @@ -398,7 +399,8 @@ static int add_uri_to_consumer(struct consumer_output *consumer, sizeof(consumer->dst.trace_path)); /* Append default trace dir */ strncat(consumer->dst.trace_path, default_trace_dir, - sizeof(consumer->dst.trace_path)); + sizeof(consumer->dst.trace_path) - + strlen(consumer->dst.trace_path) - 1); /* Flag consumer as local. */ consumer->type = CONSUMER_DST_LOCAL; break; @@ -2196,7 +2198,8 @@ int cmd_enable_consumer(int domain, struct ltt_session *session) /* Append default kernel trace dir to subdir */ strncat(ksess->consumer->subdir, DEFAULT_KERNEL_TRACE_DIR, - sizeof(ksess->consumer->subdir)); + sizeof(ksess->consumer->subdir) - + strlen(ksess->consumer->subdir) - 1); /* * @session-lock @@ -2281,7 +2284,8 @@ int cmd_enable_consumer(int domain, struct ltt_session *session) /* Append default kernel trace dir to subdir */ strncat(usess->consumer->subdir, DEFAULT_UST_TRACE_DIR, - sizeof(usess->consumer->subdir)); + sizeof(usess->consumer->subdir) - + strlen(usess->consumer->subdir) - 1); /* * @session-lock diff --git a/src/bin/lttng-sessiond/consumer.c b/src/bin/lttng-sessiond/consumer.c index c9197bd8e..b35c91195 100644 --- a/src/bin/lttng-sessiond/consumer.c +++ b/src/bin/lttng-sessiond/consumer.c @@ -560,9 +560,12 @@ int consumer_send_stream(int sock, struct consumer_output *dst, break; case CONSUMER_DST_LOCAL: /* Add stream file name to stream path */ - strncat(msg->u.stream.path_name, "/", sizeof(msg->u.stream.path_name)); + strncat(msg->u.stream.path_name, "/", + sizeof(msg->u.stream.path_name) - + strlen(msg->u.stream.path_name) - 1); strncat(msg->u.stream.path_name, msg->u.stream.name, - sizeof(msg->u.stream.path_name)); + sizeof(msg->u.stream.path_name) - + strlen(msg->u.stream.path_name) - 1); msg->u.stream.path_name[sizeof(msg->u.stream.path_name) - 1] = '\0'; /* Indicate that the stream is NOT network */ msg->u.stream.net_index = -1; diff --git a/src/bin/lttng-sessiond/main.c b/src/bin/lttng-sessiond/main.c index c0cfddb96..b9c2177fa 100644 --- a/src/bin/lttng-sessiond/main.c +++ b/src/bin/lttng-sessiond/main.c @@ -1856,7 +1856,8 @@ static int copy_session_consumer(int domain, struct ltt_session *session) } /* Append correct directory to subdir */ - strncat(consumer->subdir, dir_name, sizeof(consumer->subdir)); + strncat(consumer->subdir, dir_name, + sizeof(consumer->subdir) - strlen(consumer->subdir) - 1); DBG3("Copy session consumer subdir %s", consumer->subdir); ret = LTTCOMM_OK; diff --git a/src/common/utils.c b/src/common/utils.c index 0494b23bd..729aa76f9 100644 --- a/src/common/utils.c +++ b/src/common/utils.c @@ -70,7 +70,7 @@ char *utils_expand_path(const char *path) } /* Add end part to expanded path */ - strncat(expanded_path, end_path, PATH_MAX); + strncat(expanded_path, end_path, PATH_MAX - strlen(expanded_path) - 1); free(cut_path); return expanded_path; -- 2.34.1