From: Jérémie Galarneau Date: Sat, 14 Nov 2020 02:39:36 +0000 (-0500) Subject: Fix: unchecked buffer size for communication header X-Git-Tag: v2.13.0-rc1~423 X-Git-Url: https://git.lttng.org/?p=lttng-tools.git;a=commitdiff_plain;h=3e6e0df2f8f9f23d252c2508b6d741916dfcc4b3;hp=8a82be4c62d09a7ef4792c0eed7c7903dfac8424 Fix: unchecked buffer size for communication header A number of object de-serialization functions rely on a fixed-size communication header to create an object from a payload. A large number of those functions assume that the initial header fits in the provided buffer or payload view. Also, the functions that do validate that the header fits do so in different ways: - checking the view's size, - creating a new fixed-size view and checking the 'data' pointer. To harmonize all of those checks, the following utils are added: - lttng_buffer_view_is_valid() - lttng_payload_view_is_valid() These functions should be used whenever a fixed-size view is created (not passing -1 as the length parameter). The checks are added and/or harmonized to: - create a new 'header' view, - validate it with the corresponding *_is_valid() function, - initialize the header pointer using the header view. Signed-off-by: Jérémie Galarneau Change-Id: I763946feac714ecef4fc5bd427dab2d3fe5dc1a4 --- diff --git a/src/bin/lttng-relayd/cmd-2-11.c b/src/bin/lttng-relayd/cmd-2-11.c index 500f9ce48..cd23f22d6 100644 --- a/src/bin/lttng-relayd/cmd-2-11.c +++ b/src/bin/lttng-relayd/cmd-2-11.c @@ -87,12 +87,29 @@ int cmd_create_session_2_11(const struct lttng_buffer_view *payload, offset = header_len; session_name_view = lttng_buffer_view_from_view(payload, offset, header.session_name_len); + if (!lttng_buffer_view_is_valid(&session_name_view)) { + ERR("Invalid payload in \"cmd_create_session_2_11\": buffer too short to contain session name"); + ret = -1; + goto error; + } + offset += header.session_name_len; hostname_view = lttng_buffer_view_from_view(payload, offset, header.hostname_len); + if (!lttng_buffer_view_is_valid(&hostname_view)) { + ERR("Invalid payload in \"cmd_create_session_2_11\": buffer too short to contain hostname"); + ret = -1; + goto error; + } + offset += header.hostname_len; base_path_view = lttng_buffer_view_from_view(payload, offset, header.base_path_len); + if (header.base_path_len > 0 && !lttng_buffer_view_is_valid(&base_path_view)) { + ERR("Invalid payload in \"cmd_create_session_2_11\": buffer too short to contain base path"); + ret = -1; + goto error; + } /* Validate that names are NULL terminated. */ if (session_name_view.data[session_name_view.size - 1] != '\0') { @@ -190,9 +207,12 @@ int cmd_recv_stream_2_11(const struct lttng_buffer_view *payload, /* Validate that names are (NULL terminated. */ channel_name_view = lttng_buffer_view_from_view(payload, header_len, - header.channel_name_len); - pathname_view = lttng_buffer_view_from_view(payload, - header_len + header.channel_name_len, header.pathname_len); + header.channel_name_len); + if (!lttng_buffer_view_is_valid(&channel_name_view)) { + ERR("Invalid payload received in \"cmd_recv_stream_2_11\": buffer too short for channel name"); + ret = -1; + goto error; + } if (channel_name_view.data[channel_name_view.size - 1] != '\0') { ERR("cmd_recv_stream_2_11 channel_name is invalid (not NULL terminated)"); @@ -200,6 +220,14 @@ int cmd_recv_stream_2_11(const struct lttng_buffer_view *payload, goto error; } + pathname_view = lttng_buffer_view_from_view(payload, + header_len + header.channel_name_len, header.pathname_len); + if (!lttng_buffer_view_is_valid(&pathname_view)) { + ERR("Invalid payload received in \"cmd_recv_stream_2_11\": buffer too short for path name"); + ret = -1; + goto error; + } + if (pathname_view.data[pathname_view.size - 1] != '\0') { ERR("cmd_recv_stream_2_11 patname is invalid (not NULL terminated)"); ret = -1; diff --git a/src/bin/lttng-relayd/main.c b/src/bin/lttng-relayd/main.c index a250100d1..9f094f542 100644 --- a/src/bin/lttng-relayd/main.c +++ b/src/bin/lttng-relayd/main.c @@ -1961,7 +1961,7 @@ static int relay_recv_metadata(const struct lttcomm_relayd_hdr *recv_hdr, packet_view = lttng_buffer_view_from_view(payload, sizeof(metadata_payload_header), metadata_payload_size); - if (!packet_view.data) { + if (!lttng_buffer_view_is_valid(&packet_view)) { ERR("Invalid metadata packet length announced by header"); ret = -1; goto end_put; @@ -2667,7 +2667,6 @@ static int relay_create_trace_chunk(const struct lttcomm_relayd_hdr *recv_hdr, struct lttcomm_relayd_create_trace_chunk *msg; struct lttcomm_relayd_generic_reply reply = {}; struct lttng_buffer_view header_view; - struct lttng_buffer_view chunk_name_view; struct lttng_trace_chunk *chunk = NULL, *published_chunk = NULL; enum lttng_error_code reply_code = LTTNG_OK; enum lttng_trace_chunk_status chunk_status; @@ -2686,7 +2685,7 @@ static int relay_create_trace_chunk(const struct lttcomm_relayd_hdr *recv_hdr, } header_view = lttng_buffer_view_from_view(payload, 0, sizeof(*msg)); - if (!header_view.data) { + if (!lttng_buffer_view_is_valid(&header_view)) { ERR("Failed to receive payload of chunk creation command"); ret = -1; goto end_no_reply; @@ -2731,13 +2730,21 @@ static int relay_create_trace_chunk(const struct lttcomm_relayd_hdr *recv_hdr, if (msg->override_name_length) { const char *name; + const struct lttng_buffer_view chunk_name_view = + lttng_buffer_view_from_view(payload, + sizeof(*msg), + msg->override_name_length); + + if (!lttng_buffer_view_is_valid(&chunk_name_view)) { + ERR("Invalid payload of chunk creation command (protocol error): buffer too short for expected name length"); + ret = -1; + reply_code = LTTNG_ERR_INVALID; + goto end; + } - chunk_name_view = lttng_buffer_view_from_view(payload, - sizeof(*msg), - msg->override_name_length); name = chunk_name_view.data; - if (!name || name[msg->override_name_length - 1]) { - ERR("Failed to receive payload of chunk creation command"); + if (name[msg->override_name_length - 1]) { + ERR("Invalid payload of chunk creation command (protocol error): name is not null-terminated"); ret = -1; reply_code = LTTNG_ERR_INVALID; goto end; @@ -2872,7 +2879,7 @@ static int relay_close_trace_chunk(const struct lttcomm_relayd_hdr *recv_hdr, } header_view = lttng_buffer_view_from_view(payload, 0, sizeof(*msg)); - if (!header_view.data) { + if (!lttng_buffer_view_is_valid(&header_view)) { ERR("Failed to receive payload of chunk close command"); ret = -1; goto end_no_reply; @@ -3117,7 +3124,7 @@ static int relay_trace_chunk_exists(const struct lttcomm_relayd_hdr *recv_hdr, } header_view = lttng_buffer_view_from_view(payload, 0, sizeof(*msg)); - if (!header_view.data) { + if (!lttng_buffer_view_is_valid(&header_view)) { ERR("Failed to receive payload of chunk exists command"); ret = -1; goto end_no_reply; @@ -3169,7 +3176,7 @@ static int relay_get_configuration(const struct lttcomm_relayd_hdr *recv_hdr, uint64_t result_flags = 0; header_view = lttng_buffer_view_from_view(payload, 0, sizeof(*msg)); - if (!header_view.data) { + if (!lttng_buffer_view_is_valid(&header_view)) { ERR("Failed to receive payload of chunk close command"); ret = -1; goto end_no_reply; diff --git a/src/bin/lttng-sessiond/client.c b/src/bin/lttng-sessiond/client.c index 04846abf5..ecd31ea48 100644 --- a/src/bin/lttng-sessiond/client.c +++ b/src/bin/lttng-sessiond/client.c @@ -1334,6 +1334,11 @@ error_add_context: payload_view = lttng_buffer_view_from_dynamic_buffer( &payload, 0, name_len); + if (name_len > 0 && !lttng_buffer_view_is_valid(&payload_view)) { + ret = LTTNG_ERR_INVALID_PROTOCOL; + goto error_add_remove_tracker_value; + } + /* * Validate the value type and domains are legal for the process * attribute tracker that is specified and convert the value to diff --git a/src/bin/lttng-sessiond/cmd.c b/src/bin/lttng-sessiond/cmd.c index 86a162baf..39e824bde 100644 --- a/src/bin/lttng-sessiond/cmd.c +++ b/src/bin/lttng-sessiond/cmd.c @@ -3107,10 +3107,22 @@ enum lttng_error_code cmd_create_session(struct command_ctx *cmd_ctx, int sock, &payload, 0, cmd_ctx->lsm.u.create_session.home_dir_size); + if (cmd_ctx->lsm.u.create_session.home_dir_size > 0 && + !lttng_buffer_view_is_valid(&home_dir_view)) { + ERR("Invalid payload in \"create session\" command: buffer too short to contain home directory"); + ret_code = LTTNG_ERR_INVALID_PROTOCOL; + goto error; + } + session_descriptor_view = lttng_buffer_view_from_dynamic_buffer( &payload, cmd_ctx->lsm.u.create_session.home_dir_size, cmd_ctx->lsm.u.create_session.session_descriptor_size); + if (!lttng_buffer_view_is_valid(&session_descriptor_view)) { + ERR("Invalid payload in \"create session\" command: buffer too short to contain session descriptor"); + ret_code = LTTNG_ERR_INVALID_PROTOCOL; + goto error; + } ret = lttng_session_descriptor_create_from_buffer( &session_descriptor_view, &session_descriptor); diff --git a/src/common/actions/action.c b/src/common/actions/action.c index 86628542b..95a0c0f4d 100644 --- a/src/common/actions/action.c +++ b/src/common/actions/action.c @@ -140,15 +140,24 @@ ssize_t lttng_action_create_from_payload(struct lttng_payload_view *view, struct lttng_action **action) { ssize_t consumed_len, specific_action_consumed_len; - const struct lttng_action_comm *action_comm; action_create_from_payload_cb create_from_payload_cb; + const struct lttng_action_comm *action_comm; + const struct lttng_payload_view action_comm_view = + lttng_payload_view_from_view( + view, 0, sizeof(*action_comm)); if (!view || !action) { consumed_len = -1; goto end; } - action_comm = (const struct lttng_action_comm *) view->buffer.data; + if (!lttng_payload_view_is_valid(&action_comm_view)) { + /* Payload not large enough to contain the header. */ + consumed_len = -1; + goto end; + } + + action_comm = (const struct lttng_action_comm *) action_comm_view.buffer.data; DBG("Create action from payload: action-type=%s", lttng_action_type_string(action_comm->action_type)); diff --git a/src/common/actions/group.c b/src/common/actions/group.c index 31c6f456e..4ac239c9d 100644 --- a/src/common/actions/group.c +++ b/src/common/actions/group.c @@ -217,6 +217,11 @@ ssize_t lttng_action_group_create_from_payload( lttng_payload_view_from_view(view, consumed_len, view->buffer.size - consumed_len); + if (!lttng_payload_view_is_valid(&child_view)) { + consumed_len = -1; + goto end; + } + consumed_len_child = lttng_action_create_from_payload( &child_view, &child_action); if (consumed_len_child < 0) { diff --git a/src/common/actions/snapshot-session.c b/src/common/actions/snapshot-session.c index 757683865..8b23d58e7 100644 --- a/src/common/actions/snapshot-session.c +++ b/src/common/actions/snapshot-session.c @@ -211,18 +211,26 @@ ssize_t lttng_action_snapshot_session_create_from_payload( struct lttng_action **p_action) { ssize_t consumed_len; - const struct lttng_action_snapshot_session_comm *comm; const char *variable_data; struct lttng_action *action; enum lttng_action_status status; struct lttng_snapshot_output *snapshot_output = NULL; + const struct lttng_action_snapshot_session_comm *comm; + const struct lttng_payload_view snapshot_session_comm_view = + lttng_payload_view_from_view( + view, 0, sizeof(*comm)); action = lttng_action_snapshot_session_create(); if (!action) { goto error; } - comm = (typeof(comm)) view->buffer.data; + if (!lttng_payload_view_is_valid(&snapshot_session_comm_view)) { + /* Payload not large enough to contain the header. */ + goto error; + } + + comm = (typeof(comm)) snapshot_session_comm_view.buffer.data; variable_data = (const char *) &comm->data; consumed_len = sizeof(struct lttng_action_snapshot_session_comm); @@ -249,7 +257,7 @@ ssize_t lttng_action_snapshot_session_create_from_payload( lttng_payload_view_from_view(view, consumed_len, comm->snapshot_output_len); - if (!snapshot_output_buffer_view.buffer.data) { + if (!lttng_payload_view_is_valid(&snapshot_output_buffer_view)) { ERR("Failed to create buffer view for snapshot output."); goto error; } diff --git a/src/common/buffer-usage.c b/src/common/buffer-usage.c index ad6fb8416..5f6860800 100644 --- a/src/common/buffer-usage.c +++ b/src/common/buffer-usage.c @@ -251,17 +251,20 @@ ssize_t init_condition_from_payload(struct lttng_condition *condition, ssize_t ret, condition_size; enum lttng_condition_status status; enum lttng_domain_type domain_type; - const struct lttng_condition_buffer_usage_comm *condition_comm; const char *session_name, *channel_name; struct lttng_buffer_view names_view; + const struct lttng_condition_buffer_usage_comm *condition_comm; + const struct lttng_payload_view condition_comm_view = + lttng_payload_view_from_view( + src_view, 0, sizeof(*condition_comm)); - if (src_view->buffer.size < sizeof(*condition_comm)) { + if (!lttng_payload_view_is_valid(&condition_comm_view)) { ERR("Failed to initialize from malformed condition buffer: buffer too short to contain header"); ret = -1; goto end; } - condition_comm = (typeof(condition_comm)) src_view->buffer.data; + condition_comm = (typeof(condition_comm)) condition_comm_view.buffer.data; names_view = lttng_buffer_view_from_view(&src_view->buffer, sizeof(*condition_comm), -1); diff --git a/src/common/buffer-view.c b/src/common/buffer-view.c index 6f674f4a9..7337dbb06 100644 --- a/src/common/buffer-view.c +++ b/src/common/buffer-view.c @@ -18,6 +18,12 @@ struct lttng_buffer_view lttng_buffer_view_init( return view; } +LTTNG_HIDDEN +bool lttng_buffer_view_is_valid(const struct lttng_buffer_view *view) +{ + return view && view->data && view->size > 0; +} + LTTNG_HIDDEN struct lttng_buffer_view lttng_buffer_view_from_view( const struct lttng_buffer_view *src, size_t offset, diff --git a/src/common/buffer-view.h b/src/common/buffer-view.h index f22dc524c..88fd42dd4 100644 --- a/src/common/buffer-view.h +++ b/src/common/buffer-view.h @@ -35,6 +35,16 @@ LTTNG_HIDDEN struct lttng_buffer_view lttng_buffer_view_init( const char *src, size_t offset, ptrdiff_t len); +/** + * Checks if a buffer view is safe to access. + * + * After calling the buffer view creation functions, callers should verify + * if the resquested length (if any is explicitly provided) could be mapped + * to a new view. + */ +LTTNG_HIDDEN +bool lttng_buffer_view_is_valid(const struct lttng_buffer_view *view); + /** * Return a buffer view referencing a subset of the memory referenced by another * view. diff --git a/src/common/condition.c b/src/common/condition.c index 427c49e09..cffe6cf5e 100644 --- a/src/common/condition.c +++ b/src/common/condition.c @@ -132,16 +132,25 @@ ssize_t lttng_condition_create_from_payload( struct lttng_condition **condition) { ssize_t ret, condition_size = 0; - const struct lttng_condition_comm *condition_comm; condition_create_from_payload_cb create_from_payload = NULL; + const struct lttng_condition_comm *condition_comm; + const struct lttng_payload_view condition_comm_view = + lttng_payload_view_from_view( + view, 0, sizeof(*condition_comm)); if (!view || !condition) { ret = -1; goto end; } + if (!lttng_payload_view_is_valid(&condition_comm_view)) { + /* Payload not large enough to contain the header. */ + ret = -1; + goto end; + } + DBG("Deserializing condition from buffer"); - condition_comm = (typeof(condition_comm)) view->buffer.data; + condition_comm = (typeof(condition_comm)) condition_comm_view.buffer.data; condition_size += sizeof(*condition_comm); switch ((enum lttng_condition_type) condition_comm->condition_type) { diff --git a/src/common/evaluation.c b/src/common/evaluation.c index e936bdd91..b76d349fa 100644 --- a/src/common/evaluation.c +++ b/src/common/evaluation.c @@ -53,17 +53,24 @@ ssize_t lttng_evaluation_create_from_payload( { ssize_t ret, evaluation_size = 0; const struct lttng_evaluation_comm *evaluation_comm; - struct lttng_payload_view evaluation_view = src_view ? + struct lttng_payload_view evaluation_comm_view = + lttng_payload_view_from_view( + src_view, 0, sizeof(*evaluation_comm)); + struct lttng_payload_view evaluation_view = lttng_payload_view_from_view(src_view, - sizeof(*evaluation_comm), -1) : - (typeof(evaluation_view)) {}; + sizeof(*evaluation_comm), -1); if (!src_view || !evaluation) { ret = -1; goto end; } - evaluation_comm = (typeof(evaluation_comm)) src_view->buffer.data; + if (!lttng_payload_view_is_valid(&evaluation_comm_view)) { + ret = -1; + goto end; + } + + evaluation_comm = (typeof(evaluation_comm)) evaluation_comm_view.buffer.data; evaluation_size += sizeof(*evaluation_comm); switch ((enum lttng_condition_type) evaluation_comm->type) { diff --git a/src/common/event-rule/event-rule.c b/src/common/event-rule/event-rule.c index cdf3e0ded..b6a0e96e7 100644 --- a/src/common/event-rule/event-rule.c +++ b/src/common/event-rule/event-rule.c @@ -145,16 +145,24 @@ ssize_t lttng_event_rule_create_from_payload( struct lttng_event_rule **event_rule) { ssize_t ret, consumed = 0; - const struct lttng_event_rule_comm *event_rule_comm; event_rule_create_from_payload_cb create_from_payload = NULL; + const struct lttng_event_rule_comm *event_rule_comm; + const struct lttng_payload_view event_rule_comm_view = + lttng_payload_view_from_view( + view, 0, sizeof(*event_rule_comm)); if (!view || !event_rule) { ret = -1; goto end; } + if (!lttng_payload_view_is_valid(&event_rule_comm_view)) { + ret = -1; + goto end; + } + DBG("Deserializing event_rule from payload."); - event_rule_comm = (const struct lttng_event_rule_comm *) view->buffer.data; + event_rule_comm = (const struct lttng_event_rule_comm *) event_rule_comm_view.buffer.data; consumed += sizeof(*event_rule_comm); switch ((enum lttng_event_rule_type) event_rule_comm->event_rule_type) { diff --git a/src/common/event-rule/kprobe.c b/src/common/event-rule/kprobe.c index a5c93e653..9cc5d7c54 100644 --- a/src/common/event-rule/kprobe.c +++ b/src/common/event-rule/kprobe.c @@ -219,19 +219,15 @@ ssize_t lttng_event_rule_kprobe_create_from_payload( goto end; } - if (view->buffer.size < sizeof(*kprobe_comm)) { + current_buffer_view = lttng_buffer_view_from_view( + &view->buffer, offset, sizeof(*kprobe_comm)); + if (!lttng_buffer_view_is_valid(¤t_buffer_view)) { ERR("Failed to initialize from malformed event rule kprobe: buffer too short to contain header."); ret = -1; goto end; } - current_buffer_view = lttng_buffer_view_from_view( - &view->buffer, offset, sizeof(*kprobe_comm)); kprobe_comm = (typeof(kprobe_comm)) current_buffer_view.data; - if (!kprobe_comm) { - ret = -1; - goto end; - } rule = lttng_event_rule_kprobe_create(); if (!rule) { @@ -251,12 +247,12 @@ ssize_t lttng_event_rule_kprobe_create_from_payload( lttng_payload_view_from_view(view, offset, kprobe_comm->name_len); - name = current_payload_view.buffer.data; - if (!name) { + if (!lttng_payload_view_is_valid(¤t_payload_view)) { ret = -1; goto end; } + name = current_payload_view.buffer.data; if (!lttng_buffer_view_contains_string( ¤t_payload_view.buffer, name, kprobe_comm->name_len)) { @@ -274,6 +270,11 @@ ssize_t lttng_event_rule_kprobe_create_from_payload( lttng_payload_view_from_view(view, offset, kprobe_comm->location_len); + if (!lttng_payload_view_is_valid(¤t_payload_view)) { + ret = -1; + goto end; + } + ret = lttng_kernel_probe_location_create_from_payload( ¤t_payload_view, &location); if (ret < 0) { diff --git a/src/common/event-rule/syscall.c b/src/common/event-rule/syscall.c index ef7ccd0f9..826953c21 100644 --- a/src/common/event-rule/syscall.c +++ b/src/common/event-rule/syscall.c @@ -276,13 +276,12 @@ ssize_t lttng_event_rule_syscall_create_from_payload( current_buffer_view = lttng_buffer_view_from_view( &view->buffer, offset, sizeof(*syscall_comm)); - syscall_comm = (typeof(syscall_comm)) current_buffer_view.data; - - if (!syscall_comm) { + if (!lttng_buffer_view_is_valid(¤t_buffer_view)) { ret = -1; goto end; } + syscall_comm = (typeof(syscall_comm)) current_buffer_view.data; rule = lttng_event_rule_syscall_create(); if (!rule) { ERR("Failed to create event rule syscall"); @@ -296,12 +295,12 @@ ssize_t lttng_event_rule_syscall_create_from_payload( /* Map the pattern. */ current_buffer_view = lttng_buffer_view_from_view( &view->buffer, offset, syscall_comm->pattern_len); - pattern = current_buffer_view.data; - if (!pattern) { + if (!lttng_buffer_view_is_valid(¤t_buffer_view)) { ret = -1; goto end; } + pattern = current_buffer_view.data; if (!lttng_buffer_view_contains_string(¤t_buffer_view, pattern, syscall_comm->pattern_len)) { ret = -1; @@ -318,12 +317,12 @@ ssize_t lttng_event_rule_syscall_create_from_payload( /* Map the filter_expression. */ current_buffer_view = lttng_buffer_view_from_view(&view->buffer, offset, syscall_comm->filter_expression_len); - filter_expression = current_buffer_view.data; - if (!filter_expression) { + if (!lttng_buffer_view_is_valid(¤t_buffer_view)) { ret = -1; goto end; } + filter_expression = current_buffer_view.data; if (!lttng_buffer_view_contains_string(¤t_buffer_view, filter_expression, syscall_comm->filter_expression_len)) { diff --git a/src/common/event-rule/tracepoint.c b/src/common/event-rule/tracepoint.c index 14e4c7b01..f750af47f 100644 --- a/src/common/event-rule/tracepoint.c +++ b/src/common/event-rule/tracepoint.c @@ -603,21 +603,16 @@ ssize_t lttng_event_rule_tracepoint_create_from_payload( goto end; } - if (view->buffer.size < sizeof(*tracepoint_comm)) { + current_buffer_view = lttng_buffer_view_from_view( + &view->buffer, offset, sizeof(*tracepoint_comm)); + if (!lttng_buffer_view_is_valid(¤t_buffer_view)) { ERR("Failed to initialize from malformed event rule tracepoint: buffer too short to contain header."); ret = -1; goto end; } - current_buffer_view = lttng_buffer_view_from_view( - &view->buffer, offset, sizeof(*tracepoint_comm)); tracepoint_comm = (typeof(tracepoint_comm)) current_buffer_view.data; - if (!tracepoint_comm) { - ret = -1; - goto end; - } - if (tracepoint_comm->domain_type <= LTTNG_DOMAIN_NONE || tracepoint_comm->domain_type > LTTNG_DOMAIN_PYTHON) { /* Invalid domain value. */ @@ -667,12 +662,13 @@ ssize_t lttng_event_rule_tracepoint_create_from_payload( /* Map the pattern. */ current_buffer_view = lttng_buffer_view_from_view( &view->buffer, offset, tracepoint_comm->pattern_len); - pattern = current_buffer_view.data; - if (!pattern) { + + if (!lttng_buffer_view_is_valid(¤t_buffer_view)) { ret = -1; goto end; } + pattern = current_buffer_view.data; if (!lttng_buffer_view_contains_string(¤t_buffer_view, pattern, tracepoint_comm->pattern_len)) { ret = -1; @@ -689,12 +685,12 @@ ssize_t lttng_event_rule_tracepoint_create_from_payload( /* Map the filter_expression. */ current_buffer_view = lttng_buffer_view_from_view(&view->buffer, offset, tracepoint_comm->filter_expression_len); - filter_expression = current_buffer_view.data; - if (!filter_expression) { + if (!lttng_buffer_view_is_valid(¤t_buffer_view)) { ret = -1; goto end; } + filter_expression = current_buffer_view.data; if (!lttng_buffer_view_contains_string(¤t_buffer_view, filter_expression, tracepoint_comm->filter_expression_len)) { @@ -709,15 +705,21 @@ skip_filter_expression: for (i = 0; i < tracepoint_comm->exclusions_count; i++) { current_buffer_view = lttng_buffer_view_from_view( &view->buffer, offset, sizeof(*exclusion_len)); - exclusion_len = (typeof(exclusion_len)) current_buffer_view.data; - if (!exclusion_len) { + if (!lttng_buffer_view_is_valid(¤t_buffer_view)) { ret = -1; goto end; } + exclusion_len = (typeof(exclusion_len)) current_buffer_view.data; offset += sizeof(*exclusion_len); + current_buffer_view = lttng_buffer_view_from_view( &view->buffer, offset, *exclusion_len); + if (!lttng_buffer_view_is_valid(¤t_buffer_view)) { + ret = -1; + goto end; + } + exclusion = current_buffer_view.data; if (!lttng_buffer_view_contains_string(¤t_buffer_view, exclusion, *exclusion_len)) { diff --git a/src/common/event-rule/uprobe.c b/src/common/event-rule/uprobe.c index 8d63975ab..d9c1fc20c 100644 --- a/src/common/event-rule/uprobe.c +++ b/src/common/event-rule/uprobe.c @@ -211,24 +211,19 @@ ssize_t lttng_event_rule_uprobe_create_from_payload( goto end; } - if (view->buffer.size < sizeof(*uprobe_comm)) { - ERR("Failed to initialize from malformed event rule uprobe: buffer too short to contain header."); - ret = -1; - goto end; - } - current_buffer_view = lttng_buffer_view_from_view( &view->buffer, offset, sizeof(*uprobe_comm)); - uprobe_comm = (typeof(uprobe_comm)) current_buffer_view.data; - - if (!uprobe_comm) { + if (!lttng_buffer_view_is_valid(¤t_buffer_view)) { + ERR("Failed to initialize from malformed event rule uprobe: buffer too short to contain header"); ret = -1; goto end; } + uprobe_comm = (typeof(uprobe_comm)) current_buffer_view.data; + rule = lttng_event_rule_uprobe_create(); if (!rule) { - ERR("Failed to create event rule uprobe."); + ERR("Failed to create event rule uprobe"); ret = -1; goto end; } @@ -239,12 +234,12 @@ ssize_t lttng_event_rule_uprobe_create_from_payload( /* Map the name. */ current_buffer_view = lttng_buffer_view_from_view( &view->buffer, offset, uprobe_comm->name_len); - name = current_buffer_view.data; - if (!name) { + if (!lttng_buffer_view_is_valid(¤t_buffer_view)) { ret = -1; goto end; } + name = current_buffer_view.data; if (!lttng_buffer_view_contains_string(¤t_buffer_view, name, uprobe_comm->name_len)) { ret = -1; @@ -255,14 +250,23 @@ ssize_t lttng_event_rule_uprobe_create_from_payload( offset += uprobe_comm->name_len; /* Map the location. */ - struct lttng_payload_view current_payload_view = - lttng_payload_view_from_view(view, offset, - uprobe_comm->location_len); - ret = lttng_userspace_probe_location_create_from_payload( - ¤t_payload_view, &location); - if (ret < 0) { - ret = -1; - goto end; + { + struct lttng_payload_view current_payload_view = + lttng_payload_view_from_view(view, offset, + uprobe_comm->location_len); + + if (!lttng_payload_view_is_valid(¤t_payload_view)) { + ERR("Failed to initialize from malformed event rule uprobe: buffer too short to contain location"); + ret = -1; + goto end; + } + + ret = lttng_userspace_probe_location_create_from_payload( + ¤t_payload_view, &location); + if (ret < 0) { + ret = -1; + goto end; + } } assert(ret == uprobe_comm->location_len); diff --git a/src/common/kernel-probe.c b/src/common/kernel-probe.c index 9a84727ca..27ca02c99 100644 --- a/src/common/kernel-probe.c +++ b/src/common/kernel-probe.c @@ -433,20 +433,23 @@ ssize_t lttng_kernel_probe_location_create_from_payload( struct lttng_payload_view *view, struct lttng_kernel_probe_location **location) { - struct lttng_kernel_probe_location_comm *probe_location_comm; enum lttng_kernel_probe_location_type type; ssize_t consumed = 0; ssize_t ret; + const struct lttng_kernel_probe_location_comm *probe_location_comm; + const struct lttng_payload_view probe_location_comm_view = + lttng_payload_view_from_view( + view, 0, sizeof(*probe_location_comm)); assert(view); assert(location); - if (view->buffer.size <= sizeof(*probe_location_comm)) { + if (!lttng_payload_view_is_valid(&probe_location_comm_view)) { ret = -LTTNG_ERR_INVALID; goto end; } - probe_location_comm = (typeof(probe_location_comm)) view->buffer.data; + probe_location_comm = (typeof(probe_location_comm)) probe_location_comm_view.buffer.data; type = (enum lttng_kernel_probe_location_type) probe_location_comm->type; consumed += sizeof(*probe_location_comm); diff --git a/src/common/location.c b/src/common/location.c index 294378c75..c79f85475 100644 --- a/src/common/location.c +++ b/src/common/location.c @@ -125,11 +125,12 @@ ssize_t lttng_trace_archive_location_create_from_buffer( location_comm_view = lttng_buffer_view_from_view(view, 0, sizeof(*location_comm)); - if (!location_comm_view.data) { + if (!lttng_buffer_view_is_valid(&location_comm_view)) { goto error; } + offset += location_comm_view.size; - location_comm = (const struct lttng_trace_archive_location_comm *) view->data; + location_comm = (const struct lttng_trace_archive_location_comm *) location_comm_view.data; switch ((enum lttng_trace_archive_location_type) location_comm->type) { case LTTNG_TRACE_ARCHIVE_LOCATION_TYPE_LOCAL: @@ -138,9 +139,10 @@ ssize_t lttng_trace_archive_location_create_from_buffer( lttng_buffer_view_from_view(view, offset, location_comm->types.local.absolute_path_len); - if (!absolute_path_view.data) { + if (!lttng_buffer_view_is_valid(&absolute_path_view)) { goto error; } + if (absolute_path_view.data[absolute_path_view.size - 1] != '\0') { goto error; } @@ -163,9 +165,12 @@ ssize_t lttng_trace_archive_location_create_from_buffer( offset + hostname_view.size, location_comm->types.relay.relative_path_len); - if (!hostname_view.data || !relative_path_view.data) { + if (!lttng_buffer_view_is_valid(&hostname_view) || + !lttng_buffer_view_is_valid( + &relative_path_view)) { goto error; } + if (hostname_view.data[hostname_view.size - 1] != '\0') { goto error; } diff --git a/src/common/notification.c b/src/common/notification.c index 2544d042f..c347b3cea 100644 --- a/src/common/notification.c +++ b/src/common/notification.c @@ -78,16 +78,25 @@ ssize_t lttng_notification_create_from_payload( struct lttng_notification **notification) { ssize_t ret, notification_size = 0, condition_size, evaluation_size; - const struct lttng_notification_comm *notification_comm; struct lttng_condition *condition; struct lttng_evaluation *evaluation; + const struct lttng_notification_comm *notification_comm; + const struct lttng_payload_view notification_comm_view = + lttng_payload_view_from_view( + src_view, 0, sizeof(*notification_comm)); if (!src_view || !notification) { ret = -1; goto end; } - notification_comm = (typeof(notification_comm)) src_view->buffer.data; + if (!lttng_payload_view_is_valid(¬ification_comm_view)) { + /* Payload not large enough to contain the header. */ + ret = -1; + goto end; + } + + notification_comm = (typeof(notification_comm)) notification_comm_view.buffer.data; notification_size += sizeof(*notification_comm); { /* struct lttng_condition */ diff --git a/src/common/payload-view.c b/src/common/payload-view.c index 4a4edea14..247ecec0a 100644 --- a/src/common/payload-view.c +++ b/src/common/payload-view.c @@ -11,16 +11,22 @@ #include "payload.h" #include +LTTNG_HIDDEN +bool lttng_payload_view_is_valid(const struct lttng_payload_view *view) +{ + return view && lttng_buffer_view_is_valid(&view->buffer); +} + LTTNG_HIDDEN struct lttng_payload_view lttng_payload_view_from_payload( const struct lttng_payload *payload, size_t offset, ptrdiff_t len) { - return (struct lttng_payload_view) { + return payload ? (struct lttng_payload_view) { .buffer = lttng_buffer_view_from_dynamic_buffer( &payload->buffer, offset, len), ._fd_handles = payload->_fd_handles, - }; + } : (struct lttng_payload_view) {}; } LTTNG_HIDDEN @@ -28,13 +34,13 @@ struct lttng_payload_view lttng_payload_view_from_view( struct lttng_payload_view *view, size_t offset, ptrdiff_t len) { - return (struct lttng_payload_view) { + return view ? (struct lttng_payload_view) { .buffer = lttng_buffer_view_from_view( &view->buffer, offset, len), ._fd_handles = view->_fd_handles, ._iterator.p_fd_handles_position = view->_iterator.p_fd_handles_position ?: &view->_iterator.fd_handles_position, - }; + } : (struct lttng_payload_view) {}; } LTTNG_HIDDEN @@ -42,10 +48,10 @@ struct lttng_payload_view lttng_payload_view_from_dynamic_buffer( const struct lttng_dynamic_buffer *buffer, size_t offset, ptrdiff_t len) { - return (struct lttng_payload_view) { + return buffer ? (struct lttng_payload_view) { .buffer = lttng_buffer_view_from_dynamic_buffer( buffer, offset, len) - }; + } : (struct lttng_payload_view) {}; } LTTNG_HIDDEN @@ -53,10 +59,10 @@ struct lttng_payload_view lttng_payload_view_from_buffer_view( const struct lttng_buffer_view *view, size_t offset, ptrdiff_t len) { - return (struct lttng_payload_view) { + return view ? (struct lttng_payload_view) { .buffer = lttng_buffer_view_from_view( view, offset, len) - }; + } : (struct lttng_payload_view) {}; } LTTNG_HIDDEN diff --git a/src/common/payload-view.h b/src/common/payload-view.h index 47c2c8c91..9aa15f456 100644 --- a/src/common/payload-view.h +++ b/src/common/payload-view.h @@ -53,6 +53,16 @@ struct lttng_payload_view { } _iterator; }; +/** + * Checks if a payload view's buffer is safe to access. + * + * After calling the payload view creation functions, callers should verify + * if the resquested length (if any is explicitly provided) could be mapped + * to a new view. + */ +LTTNG_HIDDEN +bool lttng_payload_view_is_valid(const struct lttng_payload_view *view); + /** * Return a payload view referencing a subset of a payload. * diff --git a/src/common/session-consumed-size.c b/src/common/session-consumed-size.c index 046107343..e147d1e5f 100644 --- a/src/common/session-consumed-size.c +++ b/src/common/session-consumed-size.c @@ -161,19 +161,21 @@ ssize_t init_condition_from_payload(struct lttng_condition *condition, { ssize_t ret, condition_size; enum lttng_condition_status status; - const struct lttng_condition_session_consumed_size_comm *condition_comm; const char *session_name; - struct lttng_buffer_view names_view; + struct lttng_buffer_view session_name_view; + const struct lttng_condition_session_consumed_size_comm *condition_comm; + struct lttng_payload_view condition_comm_view = lttng_payload_view_from_view( + src_view, 0, sizeof(*condition_comm)); - if (src_view->buffer.size < sizeof(*condition_comm)) { + if (!lttng_payload_view_is_valid(&condition_comm_view)) { ERR("Failed to initialize from malformed condition buffer: buffer too short to contain header"); ret = -1; goto end; } - condition_comm = (typeof(condition_comm)) src_view->buffer.data; - names_view = lttng_buffer_view_from_view(&src_view->buffer, - sizeof(*condition_comm), -1); + condition_comm = (typeof(condition_comm)) condition_comm_view.buffer.data; + session_name_view = lttng_buffer_view_from_view(&src_view->buffer, + sizeof(*condition_comm), condition_comm->session_name_len); if (condition_comm->session_name_len > LTTNG_NAME_MAX) { ERR("Failed to initialize from malformed condition buffer: name exceeds LTTNG_MAX_NAME"); @@ -181,7 +183,7 @@ ssize_t init_condition_from_payload(struct lttng_condition *condition, goto end; } - if (names_view.size < condition_comm->session_name_len) { + if (!lttng_buffer_view_is_valid(&session_name_view)) { ERR("Failed to initialize from malformed condition buffer: buffer too short to contain element names"); ret = -1; goto end; @@ -195,7 +197,7 @@ ssize_t init_condition_from_payload(struct lttng_condition *condition, goto end; } - session_name = names_view.data; + session_name = session_name_view.data; if (*(session_name + condition_comm->session_name_len - 1) != '\0') { ERR("Malformed session name encountered in condition buffer"); ret = -1; diff --git a/src/common/session-descriptor.c b/src/common/session-descriptor.c index f111320e4..276c840a3 100644 --- a/src/common/session-descriptor.c +++ b/src/common/session-descriptor.c @@ -616,12 +616,12 @@ ssize_t lttng_session_descriptor_create_from_buffer( current_view = lttng_buffer_view_from_view(payload, offset, sizeof(*base_header)); - base_header = (typeof(base_header)) current_view.data; - if (!base_header) { + if (!lttng_buffer_view_is_valid(¤t_view)) { ret = -1; goto end; } + base_header = (typeof(base_header)) current_view.data; switch (base_header->type) { case LTTNG_SESSION_DESCRIPTOR_TYPE_REGULAR: case LTTNG_SESSION_DESCRIPTOR_TYPE_SNAPSHOT: @@ -632,12 +632,12 @@ ssize_t lttng_session_descriptor_create_from_buffer( current_view = lttng_buffer_view_from_view(payload, offset, sizeof(*live_header)); - live_header = (typeof(live_header)) current_view.data; - if (!live_header) { + if (!lttng_buffer_view_is_valid(¤t_view)) { ret = -1; goto end; } + live_header = (typeof(live_header)) current_view.data; live_timer_us = live_header->live_timer_us; break; } @@ -674,12 +674,12 @@ ssize_t lttng_session_descriptor_create_from_buffer( /* Map the name. */ current_view = lttng_buffer_view_from_view(payload, offset, base_header->name_len); - name = current_view.data; - if (!name) { + if (!lttng_buffer_view_is_valid(¤t_view)) { ret = -1; goto end; } + name = current_view.data; if (base_header->name_len == 1 || name[base_header->name_len - 1] || strlen(name) != base_header->name_len - 1) { @@ -705,11 +705,12 @@ skip_name: /* Map a URI. */ current_view = lttng_buffer_view_from_view(payload, offset, sizeof(*uri)); - uri = (typeof(uri)) current_view.data; - if (!uri) { + if (!lttng_buffer_view_is_valid(¤t_view)) { ret = -1; goto end; } + + uri = (typeof(uri)) current_view.data; uris[i] = zmalloc(sizeof(*uri)); if (!uris[i]) { ret = -1; diff --git a/src/common/session-rotation.c b/src/common/session-rotation.c index f6849d28a..7df5ebab0 100644 --- a/src/common/session-rotation.c +++ b/src/common/session-rotation.c @@ -207,11 +207,14 @@ ssize_t init_condition_from_payload(struct lttng_condition *condition, { ssize_t ret, condition_size; enum lttng_condition_status status; - const struct lttng_condition_session_rotation_comm *condition_comm; const char *session_name; struct lttng_buffer_view name_view; + const struct lttng_condition_session_rotation_comm *condition_comm; + struct lttng_payload_view condition_comm_view = + lttng_payload_view_from_view( + src_view, 0, sizeof(*condition_comm)); - if (src_view->buffer.size < sizeof(*condition_comm)) { + if (!lttng_payload_view_is_valid(&condition_comm_view)) { ERR("Failed to initialize from malformed condition buffer: buffer too short to contain header"); ret = -1; goto end; @@ -219,16 +222,16 @@ ssize_t init_condition_from_payload(struct lttng_condition *condition, condition_comm = (typeof(condition_comm)) src_view->buffer.data; name_view = lttng_buffer_view_from_view(&src_view->buffer, - sizeof(*condition_comm), -1); + sizeof(*condition_comm), condition_comm->session_name_len); - if (condition_comm->session_name_len > LTTNG_NAME_MAX) { - ERR("Failed to initialize from malformed condition buffer: name exceeds LTTNG_MAX_NAME"); + if (!lttng_buffer_view_is_valid(&name_view)) { + ERR("Failed to initialize from malformed condition buffer: buffer too short to contain session name"); ret = -1; goto end; } - if (name_view.size < condition_comm->session_name_len) { - ERR("Failed to initialize from malformed condition buffer: buffer too short to contain session name"); + if (condition_comm->session_name_len > LTTNG_NAME_MAX) { + ERR("Failed to initialize from malformed condition buffer: name exceeds LTTNG_MAX_NAME"); ret = -1; goto end; } @@ -347,19 +350,22 @@ ssize_t create_evaluation_from_payload( ssize_t ret, size; struct lttng_evaluation *evaluation = NULL; struct lttng_trace_archive_location *location = NULL; - const struct lttng_evaluation_session_rotation_comm *comm = - (typeof(comm)) view->buffer.data; - struct lttng_buffer_view location_view; + const struct lttng_evaluation_session_rotation_comm *comm; + struct lttng_payload_view comm_view = lttng_payload_view_from_view( + view, 0, sizeof(*comm)); - if (view->buffer.size < sizeof(*comm)) { + if (!lttng_payload_view_is_valid(&comm_view)) { goto error; } + comm = (typeof(comm)) comm_view.buffer.data; size = sizeof(*comm); if (comm->has_location) { - location_view = lttng_buffer_view_from_view( - &view->buffer, sizeof(*comm), -1); - if (!location_view.data) { + const struct lttng_buffer_view location_view = + lttng_buffer_view_from_view( + &view->buffer, sizeof(*comm), -1); + + if (!lttng_buffer_view_is_valid(&location_view)) { goto error; } diff --git a/src/common/tracker.c b/src/common/tracker.c index 29249d528..1c50d9d79 100644 --- a/src/common/tracker.c +++ b/src/common/tracker.c @@ -345,9 +345,10 @@ ssize_t lttng_process_attr_values_create_from_buffer( header_view = lttng_buffer_view_from_view( buffer_view, 0, sizeof(*header)); - if (!header_view.data) { + if (!lttng_buffer_view_is_valid(&header_view)) { goto error; } + offset = header_view.size; header = (typeof(header)) header_view.data; @@ -370,7 +371,7 @@ ssize_t lttng_process_attr_values_create_from_buffer( value_view = lttng_buffer_view_from_view( buffer_view, offset, sizeof(*value_comm)); - if (!value_view.data) { + if (!lttng_buffer_view_is_valid(&value_view)) { goto error; } @@ -382,8 +383,13 @@ ssize_t lttng_process_attr_values_create_from_buffer( value_name_view = lttng_buffer_view_from_view( buffer_view, offset, value_comm->value.name_len); + if (!lttng_buffer_view_is_valid(&value_name_view)) { + goto error; + } + offset += value_name_view.size; } + ret_code = process_attr_value_from_comm(domain, process_attr, type, &value_comm->value.integral, &value_name_view, &value); diff --git a/src/common/trigger.c b/src/common/trigger.c index 26997fd1f..785fce3fc 100644 --- a/src/common/trigger.c +++ b/src/common/trigger.c @@ -140,14 +140,23 @@ ssize_t lttng_trigger_create_from_payload( .uid = LTTNG_OPTIONAL_INIT_UNSET, .gid = LTTNG_OPTIONAL_INIT_UNSET, }; + const struct lttng_payload_view trigger_comm_view = + lttng_payload_view_from_view( + src_view, 0, sizeof(*trigger_comm)); if (!src_view || !trigger) { ret = -1; goto end; } + if (!lttng_payload_view_is_valid(&trigger_comm_view)) { + /* Payload not large enough to contain the header. */ + ret = -1; + goto end; + } + /* lttng_trigger_comm header */ - trigger_comm = (typeof(trigger_comm)) src_view->buffer.data; + trigger_comm = (typeof(trigger_comm)) trigger_comm_view.buffer.data; /* Set the trigger's creds. */ if (trigger_comm->uid > (uint64_t) ((uid_t) -1)) { @@ -164,7 +173,13 @@ ssize_t lttng_trigger_create_from_payload( /* Name. */ const struct lttng_payload_view name_view = lttng_payload_view_from_view( - src_view, offset, trigger_comm->name_length); + src_view, offset, + trigger_comm->name_length); + + if (!lttng_payload_view_is_valid(&name_view)) { + ret = -1; + goto end; + } name = name_view.buffer.data; if (!lttng_buffer_view_contains_string(&name_view.buffer, name, diff --git a/src/common/userspace-probe.c b/src/common/userspace-probe.c index 2ad8bfd91..609ffc1ac 100644 --- a/src/common/userspace-probe.c +++ b/src/common/userspace-probe.c @@ -1417,22 +1417,25 @@ int lttng_userspace_probe_location_create_from_payload( struct lttng_userspace_probe_location **location) { struct lttng_userspace_probe_location_lookup_method *lookup_method; - struct lttng_userspace_probe_location_comm *probe_location_comm; enum lttng_userspace_probe_location_type type; int consumed = 0; int ret; + struct lttng_userspace_probe_location_comm *probe_location_comm; + struct lttng_payload_view probe_location_comm_view = + lttng_payload_view_from_view( + view, 0, sizeof(*probe_location_comm)); assert(view); assert(location); lookup_method = NULL; - if (view->buffer.size <= sizeof(*probe_location_comm)) { + if (!lttng_payload_view_is_valid(&probe_location_comm_view)) { ret = -LTTNG_ERR_INVALID; goto end; } - probe_location_comm = (typeof(probe_location_comm)) view->buffer.data; + probe_location_comm = (typeof(probe_location_comm)) probe_location_comm_view.buffer.data; type = (enum lttng_userspace_probe_location_type) probe_location_comm->type; consumed += sizeof(*probe_location_comm); diff --git a/src/lib/lttng-ctl/lttng-ctl.c b/src/lib/lttng-ctl/lttng-ctl.c index 784597e20..001abffcd 100644 --- a/src/lib/lttng-ctl/lttng-ctl.c +++ b/src/lib/lttng-ctl/lttng-ctl.c @@ -2243,7 +2243,7 @@ int lttng_list_events(struct lttng_handle *handle, cmd_header_view = lttng_buffer_view_from_dynamic_buffer( &payload.buffer, 0, sizeof(*cmd_header)); - if (!cmd_header_view.data) { + if (!lttng_buffer_view_is_valid(&cmd_header_view)) { ret = -LTTNG_ERR_INVALID_PROTOCOL; goto end; } @@ -2310,6 +2310,11 @@ int lttng_list_events(struct lttng_handle *handle, payload_view.buffer.data, ext_comm->userspace_probe_location_len); + if (!lttng_payload_view_is_valid(&probe_location_view)) { + ret = -LTTNG_ERR_PROBE_LOCATION_INVAL; + goto end; + } + /* * Create a temporary userspace probe location * to determine the size needed by a "flattened" @@ -2449,6 +2454,11 @@ int lttng_list_events(struct lttng_handle *handle, payload_copy_view.buffer.data, ext_comm->userspace_probe_location_len); + if (!lttng_payload_view_is_valid(&probe_location_view)) { + ret = -LTTNG_ERR_PROBE_LOCATION_INVAL; + goto free_dynamic_buffer; + } + ret = lttng_userspace_probe_location_create_from_payload( &probe_location_view, &probe_location);