From 67d8e2ef48ea2cd334bf9efd90b58b976437a3cd Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Fri, 29 Nov 2019 16:42:24 -0500 Subject: [PATCH] buffer-view: introduce lttng_buffer_view_contains_string MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Jérémie Galarneau --- src/common/buffer-view.c | 42 +++++++++++++++++++++++++++++++++++ src/common/buffer-view.h | 17 +++++++++++++- tests/unit/Makefile.am | 10 +++++++-- tests/unit/test_buffer_view.c | 41 ++++++++++++++++++++++++++++++++++ 4 files changed, 107 insertions(+), 3 deletions(-) create mode 100644 tests/unit/test_buffer_view.c diff --git a/src/common/buffer-view.c b/src/common/buffer-view.c index 4bdb1eb7d..6f674f4a9 100644 --- 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 e8c351b3f..f22dc524c 100644 --- 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 +#include #include #include -#include 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 09de39e8c..e2c57bc8e 100644 --- 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 index 000000000..a129d4a9a --- /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 +#include + +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(); +} -- 2.34.1