From 879e14e8653bb2f9d8d83e50b983bc1fee571701 Mon Sep 17 00:00:00 2001 From: Francis Deslauriers Date: Thu, 23 May 2019 12:08:30 -0400 Subject: [PATCH] Fix: logging: possible buffer overflows MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Issue ===== Multiple possible buffer overflows on string operations using the `SET_TMP_PREFIX()` macro that uses `strcpy()` on parameter char array. Solution ======== Use a #define to set the length of the destination array and use it as the size parameter of the `strncpy()` and `strncat()` calls. Drawbacks ========= None. Notes ===== Coverity reported defects: CID 1401179 (#5 of 5): Copy into fixed size buffer (STRING_OVERFLOW) 9. fixed_size_dest: You might overrun the 64-character fixed-size string tmp_prefix by copying prefix without checking the length. CID 1401181 (#1 of 1): Copy into fixed size buffer (STRING_OVERFLOW) 5. fixed_size_dest: You might overrun the 64-character fixed-size string tmp_prefix by copying prefix without checking the length CID 1401186 (#1 of 1): Copy into fixed size buffer (STRING_OVERFLOW) 15. fixed_size_dest: You might overrun the 64-character fixed-size string tmp_prefix by copying prefix without checking the length. CID 1401192 (#2 of 2): Copy into fixed size buffer (STRING_OVERFLOW) 4. fixed_size_dest: You might overrun the 64-character fixed-size string tmp_prefix by copying prefix without checking the length. CID 1401197 (#1 of 1): Copy into fixed size buffer (STRING_OVERFLOW) 6. fixed_size_dest: You might overrun the 64-character fixed-size string tmp_prefix by copying prefix without checking the length. CID 1401198 (#4 of 4): Copy into fixed size buffer (STRING_OVERFLOW) 15. fixed_size_dest: You might overrun the 64-character fixed-size string tmp_prefix by copying prefix without checking the length. CID 1401203 (#1 of 1): Copy into fixed size buffer (STRING_OVERFLOW) 28. fixed_size_dest: You might overrun the 64-character fixed-size string tmp_prefix by copying prefix without checking the length. CID 1401212 (#1 of 1): Copy into fixed size buffer (STRING_OVERFLOW) 12. fixed_size_dest: You might overrun the 64-character fixed-size string tmp_prefix by copying prefix without checking the length. CID 1401215 (#4 of 4): Copy into fixed size buffer (STRING_OVERFLOW) 16. fixed_size_dest: You might overrun the 64-character fixed-size string tmp_prefix by copying prefix without checking the length. CID 1401221 (#2 of 2): Copy into fixed size buffer (STRING_OVERFLOW) 19. fixed_size_dest: You might overrun the 64-character fixed-size string tmp_prefix by copying prefix without checking the length. CID 1401227 (#1 of 1): Copy into fixed size buffer (STRING_OVERFLOW) 13. fixed_size_dest: You might overrun the 64-character fixed-size string tmp_prefix by copying prefix without checking the length. CID 1401230 (#3 of 3): Copy into fixed size buffer (STRING_OVERFLOW) 10. fixed_size_dest: You might overrun the 64-character fixed-size string tmp_prefix by copying prefix without checking the length. CID 1401232 (#3 of 3): Copy into fixed size buffer (STRING_OVERFLOW) 23. fixed_size_dest: You might overrun the 64-character fixed-size string tmp_prefix by copying prefix without checking the length. CID 1401234 (#5 of 5): Copy into fixed size buffer (STRING_OVERFLOW) 19. fixed_size_dest: You might overrun the 64-character fixed-size string tmp_prefix by copying prefix without checking the length. CID 1401254 (#10 of 10): Copy into fixed size buffer (STRING_OVERFLOW) 10. fixed_size_dest: You might overrun the 64-character fixed-size string tmp_prefix by copying prefix without checking the length. CID 1401257 (#1 of 1): Copy into fixed size buffer (STRING_OVERFLOW) 9. fixed_size_dest: You might overrun the 64-character fixed-size string tmp_prefix by copying prefix without checking the length. CID 1401261 (#2 of 2): Copy into fixed size buffer (STRING_OVERFLOW) 11. fixed_size_dest: You might overrun the 64-character fixed-size string tmp_prefix by copying prefix without checking the length. Reported-by: Coverity - Copy into fixed size buffer (STRING_OVERFLOW) Signed-off-by: Francis Deslauriers Change-Id: I922f1fb82a95e06b0c42627a2e57ba94debe1c5a Reviewed-on: https://review.lttng.org/c/babeltrace/+/1329 Reviewed-by: Jérémie Galarneau Tested-by: jenkins --- lib/lib-logging.c | 39 ++++++++++++++++++++------------------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/lib/lib-logging.c b/lib/lib-logging.c index 11ba9e71..0a988db3 100644 --- a/lib/lib-logging.c +++ b/lib/lib-logging.c @@ -100,10 +100,11 @@ static __thread char lib_logging_buf[LIB_LOGGING_BUF_SIZE]; #define PRFIELD_GSTRING(_expr) PRFIELD((_expr) ? (_expr)->str : NULL) +#define TMP_PREFIX_LEN 64 #define SET_TMP_PREFIX(_prefix2) \ do { \ - strcpy(tmp_prefix, prefix); \ - strcat(tmp_prefix, (_prefix2)); \ + strncpy(tmp_prefix, prefix, TMP_PREFIX_LEN); \ + strncat(tmp_prefix, (_prefix2), TMP_PREFIX_LEN); \ } while (0) static inline void format_component(char **buf_ch, bool extended, @@ -185,7 +186,7 @@ static inline void format_array_field_class(char **buf_ch, static inline void format_field_class(char **buf_ch, bool extended, const char *prefix, const struct bt_field_class *field_class) { - char tmp_prefix[64]; + char tmp_prefix[TMP_PREFIX_LEN]; BUF_APPEND(", %stype=%s", PRFIELD(bt_common_field_class_type_string(field_class->type))); @@ -472,7 +473,7 @@ static inline void format_trace_class(char **buf_ch, bool extended, static inline void format_trace(char **buf_ch, bool extended, const char *prefix, const struct bt_trace *trace) { - char tmp_prefix[64]; + char tmp_prefix[TMP_PREFIX_LEN]; if (trace->name.value) { BUF_APPEND(", %sname=\"%s\"", PRFIELD(trace->name.value)); @@ -503,7 +504,7 @@ static inline void format_stream_class(char **buf_ch, bool extended, const struct bt_stream_class *stream_class) { const struct bt_trace_class *trace_class; - char tmp_prefix[64]; + char tmp_prefix[TMP_PREFIX_LEN]; BUF_APPEND(", %sid=%" PRIu64, PRFIELD(stream_class->id)); @@ -548,7 +549,7 @@ static inline void format_event_class(char **buf_ch, bool extended, { const struct bt_stream_class *stream_class; const struct bt_trace_class *trace_class; - char tmp_prefix[64]; + char tmp_prefix[TMP_PREFIX_LEN]; BUF_APPEND(", %sid=%" PRIu64, PRFIELD(event_class->id)); @@ -605,7 +606,7 @@ static inline void format_stream(char **buf_ch, bool extended, const struct bt_stream_class *stream_class; const struct bt_trace_class *trace_class = NULL; const struct bt_trace *trace = NULL; - char tmp_prefix[64]; + char tmp_prefix[TMP_PREFIX_LEN]; BUF_APPEND(", %sid=%" PRIu64, PRFIELD(stream->id)); @@ -647,7 +648,7 @@ static inline void format_packet(char **buf_ch, bool extended, { const struct bt_stream *stream; const struct bt_trace_class *trace_class; - char tmp_prefix[64]; + char tmp_prefix[TMP_PREFIX_LEN]; if (!extended) { return; @@ -681,7 +682,7 @@ static inline void format_event(char **buf_ch, bool extended, const struct bt_stream *stream; const struct bt_trace_class *trace_class; const struct bt_stream_class *stream_class; - char tmp_prefix[64]; + char tmp_prefix[TMP_PREFIX_LEN]; if (!extended) { return; @@ -742,7 +743,7 @@ static inline void format_event(char **buf_ch, bool extended, static inline void format_clock_class(char **buf_ch, bool extended, const char *prefix, const struct bt_clock_class *clock_class) { - char tmp_prefix[64]; + char tmp_prefix[TMP_PREFIX_LEN]; if (clock_class->name.value) { BUF_APPEND(", %sname=\"%s\"", PRFIELD(clock_class->name.value)); @@ -781,7 +782,7 @@ static inline void format_clock_class(char **buf_ch, bool extended, static inline void format_clock_snapshot(char **buf_ch, bool extended, const char *prefix, const struct bt_clock_snapshot *clock_snapshot) { - char tmp_prefix[64]; + char tmp_prefix[TMP_PREFIX_LEN]; BUF_APPEND(", %svalue=%" PRIu64 ", %sns-from-origin=%" PRId64, PRFIELD(clock_snapshot->value_cycles), PRFIELD(clock_snapshot->ns_from_origin)); @@ -869,7 +870,7 @@ static inline void format_value(char **buf_ch, bool extended, static inline void format_message(char **buf_ch, bool extended, const char *prefix, const struct bt_message *msg) { - char tmp_prefix[64]; + char tmp_prefix[TMP_PREFIX_LEN]; BUF_APPEND(", %stype=%s", PRFIELD(bt_message_type_string(msg->type))); @@ -1008,7 +1009,7 @@ static inline void format_component_class(char **buf_ch, bool extended, const char *prefix, const struct bt_component_class *comp_class) { - char tmp_prefix[64]; + char tmp_prefix[TMP_PREFIX_LEN]; BUF_APPEND(", %stype=%s, %sname=\"%s\"", PRFIELD(bt_component_class_type_string(comp_class->type)), @@ -1035,7 +1036,7 @@ static inline void format_component_class(char **buf_ch, bool extended, static inline void format_component(char **buf_ch, bool extended, const char *prefix, const struct bt_component *component) { - char tmp_prefix[64]; + char tmp_prefix[TMP_PREFIX_LEN]; BUF_APPEND(", %sname=\"%s\"", PRFIELD_GSTRING(component->name)); @@ -1064,7 +1065,7 @@ static inline void format_component(char **buf_ch, bool extended, static inline void format_port(char **buf_ch, bool extended, const char *prefix, const struct bt_port *port) { - char tmp_prefix[64]; + char tmp_prefix[TMP_PREFIX_LEN]; BUF_APPEND(", %stype=%s, %sname=\"%s\"", PRFIELD(bt_port_type_string(port->type)), @@ -1083,7 +1084,7 @@ static inline void format_port(char **buf_ch, bool extended, static inline void format_connection(char **buf_ch, bool extended, const char *prefix, const struct bt_connection *connection) { - char tmp_prefix[64]; + char tmp_prefix[TMP_PREFIX_LEN]; if (!extended) { return; @@ -1105,7 +1106,7 @@ static inline void format_connection(char **buf_ch, bool extended, static inline void format_graph(char **buf_ch, bool extended, const char *prefix, const struct bt_graph *graph) { - char tmp_prefix[64]; + char tmp_prefix[TMP_PREFIX_LEN]; BUF_APPEND(", %sis-canceled=%d, %scan-consume=%d, " "%sconfig-state=%s", @@ -1143,7 +1144,7 @@ static inline void format_message_iterator(char **buf_ch, const struct bt_message_iterator *iterator) { const char *type; - char tmp_prefix[64]; + char tmp_prefix[TMP_PREFIX_LEN]; if (iterator->type == BT_MESSAGE_ITERATOR_TYPE_SELF_COMPONENT_PORT_INPUT) { type = "BT_MESSAGE_ITERATOR_TYPE_SELF_COMPONENT_PORT_INPUT"; @@ -1207,7 +1208,7 @@ static inline void format_message_iterator(char **buf_ch, static inline void format_plugin(char **buf_ch, bool extended, const char *prefix, const struct bt_plugin *plugin) { - char tmp_prefix[64]; + char tmp_prefix[TMP_PREFIX_LEN]; BUF_APPEND(", %stype=%s", PRFIELD(bt_plugin_type_string(plugin->type))); -- 2.34.1