buffer-view: introduce lttng_buffer_view_contains_string

This function can be used to safely validate that a string in a buffer
view has the length we expect.  The goal is to avoid doing any reads
outside the buffer view, whatever the input is, considering that the
buffer and advertised length of the string are untrusted data.

It will be used by subsequent patches to deserialize strings from
received buffers.

Change-Id: I8afc4b6f9334e035e0ef02cb96b157df59d8107d
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

diff --git a/src/common/buffer-view.c b/src/common/buffer-view.c
index 4bdb1eb7d75d48be514689200954d69c44a7809b..6f674f4a9828f857ada1a5e2fc3038d00ba343ba 100644 (file)
--- a/src/common/buffer-view.c
+++ b/src/common/buffer-view.c
@@ -67,3 +67,45 @@ struct lttng_buffer_view lttng_buffer_view_from_dynamic_buffer(
 end:
        return view;
 }
+
+LTTNG_HIDDEN
+bool lttng_buffer_view_contains_string(const struct lttng_buffer_view *buf,
+               const char *str,
+               size_t len_with_null_terminator)
+{
+       const char *past_buf_end;
+       size_t max_str_len_with_null_terminator;
+       size_t str_len;
+       bool ret;
+
+       past_buf_end = buf->data + buf->size;
+
+       /* Is the start of the string in the buffer view? */
+       if (str < buf->data || str >= past_buf_end) {
+               ret = false;
+               goto end;
+       }
+
+       /*
+        * Max length the string could have to fit in the buffer, including
+        * NULL terminator.
+        */
+       max_str_len_with_null_terminator = past_buf_end - str;
+
+       /* Could the string even fit in the buffer? */
+       if (len_with_null_terminator > max_str_len_with_null_terminator) {
+               ret = false;
+               goto end;
+       }
+
+       str_len = lttng_strnlen(str, max_str_len_with_null_terminator);
+       if (str_len != (len_with_null_terminator - 1)) {
+               ret = false;
+               goto end;
+       }
+
+       ret = true;
+
+end:
+       return ret;
+}

diff --git a/src/common/buffer-view.h b/src/common/buffer-view.h
index e8c351b3f994974a847bf4936812ad073b445ef4..f22dc524c743afbdb0f9e2ec52e3d3405ff69f8e 100644 (file)
--- a/src/common/buffer-view.h
+++ b/src/common/buffer-view.h
@@ -8,9 +8,10 @@
 #ifndef LTTNG_BUFFER_VIEW_H
 #define LTTNG_BUFFER_VIEW_H
 
+#include <common/macros.h>
+#include <stdbool.h>
 #include <stddef.h>
 #include <stdint.h>
-#include <common/macros.h>
 
 struct lttng_dynamic_buffer;
 
@@ -70,4 +71,18 @@ struct lttng_buffer_view lttng_buffer_view_from_dynamic_buffer(
                const struct lttng_dynamic_buffer *src, size_t offset,
                ptrdiff_t len);
 
+/**
+ * Verify that `buf` contains a string starting at `str` of length
+ * `len_with_null_terminator`.
+ *
+ * @buf                                The buffer view
+ * @str                                The start of the string
+ * @len_with_null_terminator   Expected length of the string, including the
+ *                             NULL terminator.
+ */
+LTTNG_HIDDEN
+bool lttng_buffer_view_contains_string(const struct lttng_buffer_view *buf,
+               const char *str,
+               size_t len_with_null_terminator);
+
 #endif /* LTTNG_BUFFER_VIEW_H */

diff --git a/tests/unit/Makefile.am b/tests/unit/Makefile.am
index 09de39e8cc99d3a22ee4378cc340484ad12e35a9..e2c57bc8e7d68b28c1b0c3e08c18cd3cec06ce0e 100644 (file)
--- a/tests/unit/Makefile.am
+++ b/tests/unit/Makefile.am
@@ -21,7 +21,8 @@ TESTS = test_kernel_data \
        test_relayd_backward_compat_group_by_session \
        ini_config/test_ini_config \
        test_fd_tracker \
-       test_uuid
+       test_uuid \
+       test_buffer_view
 
 LIBTAP=$(top_builddir)/tests/utils/tap/libtap.la
 
@@ -39,7 +40,8 @@ noinst_PROGRAMS = test_uri test_session test_kernel_data \
                   test_utils_expand_path test_utils_compat_poll \
                   test_string_utils test_notification test_directory_handle \
                   test_relayd_backward_compat_group_by_session \
-                  test_fd_tracker test_uuid
+                  test_fd_tracker test_uuid \
+                  test_buffer_view
 
 if HAVE_LIBLTTNG_UST_CTL
 noinst_PROGRAMS += test_ust_data
@@ -195,3 +197,7 @@ test_fd_tracker_LDADD = $(LIBTAP) $(LIBFDTRACKER) $(DL_LIBS) -lurcu $(LIBCOMMON)
 # uuid unit test
 test_uuid_SOURCES = test_uuid.c
 test_uuid_LDADD = $(LIBTAP) $(LIBCOMMON)
+
+# buffer view unit test
+test_buffer_view_SOURCES = test_buffer_view.c
+test_buffer_view_LDADD = $(LIBTAP) $(LIBCOMMON)

diff --git a/tests/unit/test_buffer_view.c b/tests/unit/test_buffer_view.c
new file mode 100644 (file)
index 0000000..a129d4a
--- /dev/null
+++ b/tests/unit/test_buffer_view.c
@@ -0,0 +1,41 @@
+/*
+ * Copyright (C) 2020 EfficiOS, inc.
+ *
+ * SPDX-License-Identifier: GPL-2.0-only
+ *
+ */
+
+
+#include <common/buffer-view.h>
+#include <tap/tap.h>
+
+static const int TEST_COUNT = 5;
+
+/* For error.h */
+int lttng_opt_quiet = 1;
+int lttng_opt_verbose;
+int lttng_opt_mi;
+
+static void test_contains_string(void)
+{
+       const char buf[] = {'A', 'l', 'l', 'o', '\0'};
+       struct lttng_buffer_view view = lttng_buffer_view_init(buf, 0, 5);
+       struct lttng_buffer_view view_minus_one =
+                       lttng_buffer_view_init(buf, 0, 4);
+
+       ok1(!lttng_buffer_view_contains_string(&view, buf, 4));
+       ok1(lttng_buffer_view_contains_string(&view, buf, 5));
+       ok1(!lttng_buffer_view_contains_string(&view, buf, 6));
+
+       ok1(!lttng_buffer_view_contains_string(&view_minus_one, buf, 4));
+       ok1(!lttng_buffer_view_contains_string(&view_minus_one, buf, 5));
+}
+
+int main(void)
+{
+       plan_tests(TEST_COUNT);
+
+       test_contains_string();
+
+       return exit_status();
+}