Avoid memory corruption behind buffer wp in function dlt_getloginfo_conv_ascii_to_id (#411)

* Avoid memory corruption behind buffer wp in function dlt_getloginfo_conv_ascii_to_id

- Introduced new function dlt_getloginfo_conv_ascii_to_string for '\0' terminated strings
- Avoid printing garbage characters in dlt-control after APID and CTID (which are not null terminated anymore)
- Added unit test for dlt_client_parse_get_log_info_resp_text and dlt_getloginfo_conv_ascii_to_string
- Use dlt_getloginfo_conv_ascii_to_string to get '\0' terminated for app_description and context_description

Signed-off-by: Michael Methner <mmethner@de.adit-jv.com>

5 files changed, 131 insertions, 13 deletions

diff --git a/include/dlt/dlt_common.h b/include/dlt/dlt_common.h
index 100a481..ead124f 100644
--- a/include/dlt/dlt_common.h
+++ b/include/dlt/dlt_common.h
@@ -1631,15 +1631,28 @@ int16_t dlt_getloginfo_conv_ascii_to_uint16_t(char *rp, int *rp_count);
  */
 int16_t dlt_getloginfo_conv_ascii_to_int16_t(char *rp, int *rp_count);
 
+
+/**
+ * Convert get log info from ASCII to string (with '\0' termination)
+ *
+ * @param rp        char
+ * @param rp_count  int
+ * @param wp        char Array needs to be 1 byte larger than len to store '\0'
+ * @param len       int
+ */
+void dlt_getloginfo_conv_ascii_to_string(char *rp, int *rp_count, char *wp, int len);
+
+
 /**
- * Convert get log info from ASCII to ID
+ * Convert get log info from ASCII to ID (without '\0' termination)
  *
  * @param rp        char
  * @param rp_count  int
  * @param wp        char
  * @param len       int
+ * @return position of last read character in wp
  */
-void dlt_getloginfo_conv_ascii_to_id(char *rp, int *rp_count, char *wp, int len);
+int dlt_getloginfo_conv_ascii_to_id(char *rp, int *rp_count, char *wp, int len);
 
 /**
  * Convert from hex ASCII to binary
diff --git a/src/console/dlt-control.c b/src/console/dlt-control.c
index 7bf091a..8fbf9f0 100644
--- a/src/console/dlt-control.c
+++ b/src/console/dlt-control.c
@@ -212,9 +212,9 @@ void dlt_process_get_log_info(void)
             dlt_print_id(apid, app.app_id);
 
             if (app.app_description != 0)
-                printf("APID:%4s %s\n", apid, app.app_description);
+                printf("APID:%4.4s %s\n", apid, app.app_description);
             else
-                printf("APID:%4s \n", apid);
+                printf("APID:%4.4s \n", apid);
 
             for (j = 0; j < app.count_context_ids; j++) {
                 con = app.context_id_info[j];
@@ -222,13 +222,13 @@ void dlt_process_get_log_info(void)
                 dlt_print_id(ctid, con.context_id);
 
                 if (con.context_description != 0)
-                    printf("CTID:%4s %2d %2d %s\n",
+                    printf("CTID:%4.4s %2d %2d %s\n",
                         ctid,
                         con.log_level,
                         con.trace_status,
                         con.context_description);
                 else
-                    printf("CTID:%4s %2d %2d\n",
+                    printf("CTID:%4.4s %2d %2d\n",
                         ctid,
                         con.log_level,
                         con.trace_status);
diff --git a/src/lib/dlt_client.c b/src/lib/dlt_client.c
index 245ebef..c032fd4 100644
--- a/src/lib/dlt_client.c
+++ b/src/lib/dlt_client.c
@@ -1332,7 +1332,7 @@ DltReturnValue dlt_client_parse_get_log_info_resp_text(DltServiceGetLogInfoRespo
                     return DLT_RETURN_ERROR;
                 }
 
-                dlt_getloginfo_conv_ascii_to_id(rp,
+                dlt_getloginfo_conv_ascii_to_string(rp,
                                                 &rp_count,
                                                 con->context_description,
                                                 con->len_context_description);
@@ -1352,7 +1352,7 @@ DltReturnValue dlt_client_parse_get_log_info_resp_text(DltServiceGetLogInfoRespo
                 return DLT_RETURN_ERROR;
             }
 
-            dlt_getloginfo_conv_ascii_to_id(rp,
+            dlt_getloginfo_conv_ascii_to_string(rp,
                                             &rp_count,
                                             app->app_description,
                                             app->len_app_description);
diff --git a/src/shared/dlt_common.c b/src/shared/dlt_common.c
index 2ab6ed9..4dfbc20 100644
--- a/src/shared/dlt_common.c
+++ b/src/shared/dlt_common.c
@@ -4084,17 +4084,31 @@ int16_t dlt_getloginfo_conv_ascii_to_int16_t(char *rp, int *rp_count)
     return (signed char)strtol(num_work, &endptr, 16);
 }
 
-void dlt_getloginfo_conv_ascii_to_id(char *rp, int *rp_count, char *wp, int len)
+void dlt_getloginfo_conv_ascii_to_string(char *rp, int *rp_count, char *wp, int len)
+{
+    if ((rp == NULL ) || (rp_count == NULL ) || (wp == NULL ))
+        return;
+    /* ------------------------------------------------------
+     *  from: [72 65 6d 6f ] -> to: [0x72,0x65,0x6d,0x6f,0x00]
+     *  ------------------------------------------------------ */
+
+    int count = dlt_getloginfo_conv_ascii_to_id(rp, rp_count, wp, len);
+    *(wp + count) = '\0';
+
+    return;
+}
+
+int dlt_getloginfo_conv_ascii_to_id(char *rp, int *rp_count, char *wp, int len)
 {
     char number16[3] = { 0 };
     char *endptr;
     int count;
 
     if ((rp == NULL) || (rp_count == NULL) || (wp == NULL))
-        return;
+        return 0;
 
     /* ------------------------------------------------------
-     *  from: [72 65 6d 6f ] -> to: [0x72,0x65,0x6d,0x6f,0x00]
+     *  from: [72 65 6d 6f ] -> to: [0x72,0x65,0x6d,0x6f]
      *  ------------------------------------------------------ */
     for (count = 0; count < len; count++) {
         number16[0] = *(rp + *rp_count + 0);
@@ -4103,8 +4117,7 @@ void dlt_getloginfo_conv_ascii_to_id(char *rp, int *rp_count, char *wp, int len)
         *rp_count += 3;
     }
 
-    *(wp + count) = 0;
-    return;
+    return count;
 }
 
 void dlt_hex_ascii_to_binary(const char *ptr, uint8_t *binary, int *size)
diff --git a/tests/gtest_dlt_common.cpp b/tests/gtest_dlt_common.cpp
index 9a375ce..18830be 100644
--- a/tests/gtest_dlt_common.cpp
+++ b/tests/gtest_dlt_common.cpp
@@ -34,6 +34,8 @@ extern "C"
 #include "dlt-daemon_cfg.h"
 #include "dlt_user_cfg.h"
 #include "dlt_version.h"
+#include "dlt_client.h"
+#include "dlt_protocol.h"
 
 int dlt_buffer_increase_size(DltBuffer *);
 int dlt_buffer_minimize_size(DltBuffer *);
@@ -4260,7 +4262,97 @@ TEST(dlt_get_minor_version, nullpointer)
 /* End Method:dlt_common::dlt_get_minor_version */
 
 
+TEST(dlt_client_parse_get_log_info_resp_text, normal)
+{
+    char input[] = "get_log_info, 07, 02 00 4c 4f 47 00 03 00 54 45 53 54 ff ff 18 00 54 65 73 74 20 43 6f 6e 74 65 78 74 20 66 6f 72 20 4c 6f 67 67 69 6e 67 54 53 31 00 ff ff 1b 00 54 65 73 74 20 43 6f 6e 74 65 78 74 31 20 66 6f 72 20 69 6e 6a 65 63 74 69 6f 6e 54 53 32 00 ff ff 1b 00 54 65 73 74 20 43 6f 6e 74 65 78 74 32 20 66 6f 72 20 69 6e 6a 65 63 74 69 6f 6e 1c 00 54 65 73 74 20 41 70 70 6c 69 63 61 74 69 6f 6e 20 66 6f 72 20 4c 6f 67 67 69 6e 67 53 59 53 00 02 00 4a 4f 55 52 ff ff 0f 00 4a 6f 75 72 6e 61 6c 20 41 64 61 70 74 65 72 4d 47 52 00 ff ff 22 00 43 6f 6e 74 65 78 74 20 6f 66 20 6d 61 69 6e 20 64 6c 74 20 73 79 73 74 65 6d 20 6d 61 6e 61 67 65 72 12 00 44 4c 54 20 53 79 73 74 65 6d 20 4d 61 6e 61 67 65 72 72 65 6d 6f";
+    /* expected output:
+     * APID:LOG- Test Application for Logging
+     * CTID:TEST -1 -1 Test Context for Logging
+     * CTID:TS1- -1 -1 Test Context1 for injection
+     * CTID:TS2- -1 -1 Test Context2 for injection
+     * APID:SYS- DLT System Manager
+     * CTID:JOUR -1 -1 Journal Adapter
+     * CTID:MGR- -1 -1 Context of main dlt system manager
+     */
+
+    DltServiceGetLogInfoResponse * resp = (DltServiceGetLogInfoResponse *)malloc(sizeof(DltServiceGetLogInfoResponse ));
+    DltReturnValue ret = (DltReturnValue) dlt_set_loginfo_parse_service_id(
+            input, &resp->service_id, &resp->status);
+    EXPECT_EQ(DLT_RETURN_OK, ret);
+    EXPECT_EQ(DLT_SERVICE_ID_GET_LOG_INFO, resp->service_id);
+    EXPECT_EQ(GET_LOG_INFO_STATUS_MAX, resp->status);
+
+    ret = dlt_client_parse_get_log_info_resp_text(resp, input);
+    EXPECT_EQ(DLT_RETURN_OK, ret);
+
+    EXPECT_EQ(2,resp->log_info_type.count_app_ids);
+
+    EXPECT_EQ(0, memcmp( "LOG", resp->log_info_type.app_ids[0].app_id,4));
+    EXPECT_EQ(0, strcmp("Test Application for Logging", resp->log_info_type.app_ids[0].app_description));
+    EXPECT_EQ(28, resp->log_info_type.app_ids[0].len_app_description);
+
+    EXPECT_EQ(3, resp->log_info_type.app_ids[0].count_context_ids);
+
+    EXPECT_EQ(0, memcmp( "TEST", resp->log_info_type.app_ids[0].context_id_info[0].context_id,4));
+    EXPECT_EQ(0, strcmp( "Test Context for Logging",resp->log_info_type.app_ids[0].context_id_info[0].context_description ));
+    EXPECT_EQ(24,resp->log_info_type.app_ids[0].context_id_info[0].len_context_description);
+    EXPECT_EQ(-1,resp->log_info_type.app_ids[0].context_id_info[0].log_level);
+    EXPECT_EQ(-1,resp->log_info_type.app_ids[0].context_id_info[0].trace_status);
+
+    EXPECT_EQ(0, memcmp( "TS1", resp->log_info_type.app_ids[0].context_id_info[1].context_id,4));
+    EXPECT_EQ(0, strcmp( "Test Context1 for injection",resp->log_info_type.app_ids[0].context_id_info[1].context_description ));
+    EXPECT_EQ(27,resp->log_info_type.app_ids[0].context_id_info[1].len_context_description);
+    EXPECT_EQ(-1,resp->log_info_type.app_ids[0].context_id_info[1].log_level);
+    EXPECT_EQ(-1,resp->log_info_type.app_ids[0].context_id_info[1].trace_status);
+
+    EXPECT_EQ(0, memcmp( "TS2", resp->log_info_type.app_ids[0].context_id_info[2].context_id,4));
+    EXPECT_EQ(0, strcmp( "Test Context2 for injection",resp->log_info_type.app_ids[0].context_id_info[2].context_description ));
+    EXPECT_EQ(27,resp->log_info_type.app_ids[0].context_id_info[2].len_context_description);
+    EXPECT_EQ(-1,resp->log_info_type.app_ids[0].context_id_info[2].log_level);
+    EXPECT_EQ(-1,resp->log_info_type.app_ids[0].context_id_info[2].trace_status);
+
+    EXPECT_EQ(0, memcmp( "SYS", resp->log_info_type.app_ids[1].app_id,4));
+    EXPECT_EQ(0, strcmp("DLT System Manager", resp->log_info_type.app_ids[1].app_description));
+    EXPECT_EQ(18, resp->log_info_type.app_ids[1].len_app_description);
+
+    EXPECT_EQ(2, resp->log_info_type.app_ids[1].count_context_ids);
 
+    EXPECT_EQ(0, memcmp( "JOUR", resp->log_info_type.app_ids[1].context_id_info[0].context_id,4));
+    EXPECT_EQ(0, strcmp( "Journal Adapter",resp->log_info_type.app_ids[1].context_id_info[0].context_description ));
+    EXPECT_EQ(15,resp->log_info_type.app_ids[1].context_id_info[0].len_context_description);
+    EXPECT_EQ(-1,resp->log_info_type.app_ids[1].context_id_info[0].log_level);
+    EXPECT_EQ(-1,resp->log_info_type.app_ids[1].context_id_info[0].trace_status);
+
+    EXPECT_EQ(0, memcmp( "MGR", resp->log_info_type.app_ids[1].context_id_info[1].context_id,4));
+    EXPECT_EQ(0, strcmp( "Context of main dlt system manager",resp->log_info_type.app_ids[1].context_id_info[1].context_description ));
+    EXPECT_EQ(34,resp->log_info_type.app_ids[1].context_id_info[1].len_context_description);
+    EXPECT_EQ(-1,resp->log_info_type.app_ids[1].context_id_info[1].log_level);
+    EXPECT_EQ(-1,resp->log_info_type.app_ids[1].context_id_info[1].trace_status);
+
+    ret = (DltReturnValue)dlt_client_cleanup_get_log_info(resp);
+    EXPECT_EQ(DLT_RETURN_OK,ret);
+}
+
+TEST(dlt_getloginfo_conv_ascii_to_string, normal)
+{
+    /* NULL-Pointer, expect exeption */
+    char rp_1[] = "72 65 6d 6f";
+    char rp_2[] = "123456789 72 65 6d 6f";
+    int rp_count = 0;
+    char rp_str[] =
+    { 0x72, 0x65, 0x6d, 0x6f, 0x00 };
+    char wp[5];
+
+    dlt_getloginfo_conv_ascii_to_string(rp_1, &rp_count, wp, 4);
+    EXPECT_EQ(0, strcmp(rp_str, wp));
+    EXPECT_EQ(12, rp_count);
+
+    rp_count = 10;
+    dlt_getloginfo_conv_ascii_to_string(rp_2, &rp_count, wp, 4);
+    EXPECT_EQ(0, strcmp(rp_str, wp));
+    EXPECT_EQ(22, rp_count);
+
+}
 
 /*##############################################################################################################################*/
 /*##############################################################################################################################*/