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 <christian.babeux@efficios.com>
Signed-off-by: David Goulet <dgoulet@efficios.com>

diff --git a/src/bin/lttng-sessiond/cmd.c b/src/bin/lttng-sessiond/cmd.c
index d36966322fc7278617109b9df471bda8196ea5cc..1b1f87b8e5f30e8a0b74877b246af59ba71d250d 100644 (file)
--- 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 c9197bd8ecf9afc682f3b9e43740cf7fe0298aca..b35c91195391b6c106e277443dd4a3af60d7cbc8 100644 (file)
--- 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 c0cfddb96166840bfe3effdfe0e7dcf36566a7d6..b9c2177fa4f24b9fb31a7f75c7744ebbf6ba359d 100644 (file)
--- 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 0494b23bdcd0bd356f45c0f4f8c5bf8a0f99c871..729aa76f9fd161ddeefbafc19db6374e3da14e54 100644 (file)
--- 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;