process user message: Fix bound handling

On context and application registration, the data needs to be read in 2
parts. If the first part was at the end of the receiver buffer, it was
wrongly removed whereas the complete data is not full received and
therefore interpretable. This has to be fixed by not removing the data
before everything is checked.

Thus, while readjusting the buffer to the first found header, the offset
was not properly updated.

Signed-off-by: Frederic Berat <fberat@de.adit-jv.com>

Process User Messages: Fix buffer handling

In case application and context registration the buffer need to be read
two times. But before removing the data, the data need to be readjusted
to its origin. This was missing in a previous fix.

Furthermore, in case of corrupted description field, applications and
contexts will be registered anyway with the dummy description "Unknown".

Signed-off-by: Christoph Lipka <clipka@jp.adit-jv.com>

3 files changed, 114 insertions, 26 deletions

diff --git a/include/dlt/dlt_common.h b/include/dlt/dlt_common.h
index b6a83a8..ba2324e 100644
--- a/include/dlt/dlt_common.h
+++ b/include/dlt/dlt_common.h
@@ -321,6 +321,11 @@ enum {
 #define DLT_HEADER_SHOW_NOARG      0x0200
 #define DLT_HEADER_SHOW_ALL        0xFFFF
 
+/* dlt_receiver_check_and_get flags */
+#define DLT_RCV_NONE        0
+#define DLT_RCV_SKIP_HEADER (1 << 0)
+#define DLT_RCV_REMOVE      (1 << 1)
+
 /**
  * Maximal length of mounted path
  */
diff --git a/src/daemon/dlt-daemon.c b/src/daemon/dlt-daemon.c
index 2444e4c..d3df90f 100644
--- a/src/daemon/dlt-daemon.c
+++ b/src/daemon/dlt-daemon.c
@@ -1965,14 +1965,14 @@ int dlt_daemon_process_user_messages(DltDaemon *daemon,
         dlt_daemon_process_user_message_func func = NULL;
 
         offset = 0;
-        userheader = (DltUserHeader*) (receiver->buf + offset);
+        userheader = (DltUserHeader *)(receiver->buf + offset);
 
         while (!dlt_user_check_userheader(userheader) &&
                (offset + min_size <= receiver->bytesRcvd))
         /* resync if necessary */
         {
-            userheader = (DltUserHeader*) (receiver->buf + offset);
             offset++;
+            userheader = (DltUserHeader *)(receiver->buf + offset);
         }
 
         /* Check for user header pattern */
@@ -2040,7 +2040,10 @@ int dlt_daemon_process_user_message_overflow(DltDaemon *daemon,
         return -1;
     }
 
-    if (dlt_receiver_check_and_get(rec, &userpayload, len, 1) < 0)
+    if (dlt_receiver_check_and_get(rec,
+                                   &userpayload,
+                                   len,
+                                   DLT_RCV_SKIP_HEADER | DLT_RCV_REMOVE) < 0)
     {
         /* Not enough bytes received */
         return -1;
@@ -2089,10 +2092,12 @@ int dlt_daemon_process_user_message_register_application(DltDaemon *daemon,
                                                          int verbose)
 {
     uint32_t len = sizeof(DltUserControlMsgRegisterApplication);
+    int to_remove = 0;
     DltDaemonApplication *application = NULL;
     char local_str[DLT_DAEMON_TEXTBUFSIZE] = { '\0' };
     char description[DLT_DAEMON_DESCSIZE + 1] = { '\0' };
     DltUserControlMsgRegisterApplication userapp;
+    char *origin;
 
     PRINT_FUNCTION_VERBOSE(verbose);
 
@@ -2107,18 +2112,50 @@ int dlt_daemon_process_user_message_register_application(DltDaemon *daemon,
         return -1;
     }
 
-    if (dlt_receiver_check_and_get(rec, &userapp, len, 1) < 0)
+    memset(&userapp, 0, sizeof(DltUserControlMsgRegisterApplication));
+    origin = rec->buf;
+
+    /* We shall not remove data before checking that everything is there. */
+    to_remove = dlt_receiver_check_and_get(rec,
+                                           &userapp,
+                                           len,
+                                           DLT_RCV_SKIP_HEADER);
+    if (to_remove < 0)
     {
         /* Not enough bytes received */
         return -1;
     }
 
     len = userapp.description_length;
+    if (len > DLT_DAEMON_DESCSIZE)
+    {
+        len = DLT_DAEMON_DESCSIZE;
+        dlt_log(LOG_WARNING, "Application description exceeds limit\n");
+    }
+
+    /* adjust buffer pointer */
+    rec->buf += to_remove + sizeof(DltUserHeader);
 
-    if ((len > DLT_DAEMON_DESCSIZE) ||
-        (dlt_receiver_check_and_get(rec, description, len, 0) < 0))
+    if (dlt_receiver_check_and_get(rec, description, len, DLT_RCV_NONE) < 0)
     {
         dlt_log(LOG_ERR, "Unable to get application description\n");
+        /* in case description was not readable, set dummy description */
+        strncpy(description, "Unknown", sizeof("Unknown"));
+        /* unknown len of original description, set to 0 to not remove in next
+         * step. Because message buffer is re-adjusted the corrupted description
+         * is ignored. */
+        len = 0;
+    }
+
+    /* adjust to_remove */
+    to_remove += sizeof(DltUserHeader) + len;
+    /* point to begin of message */
+    rec->buf = origin;
+
+    /* We can now remove data. */
+    if (dlt_receiver_remove(rec, to_remove) != DLT_RETURN_OK)
+    {
+        dlt_log(LOG_WARNING,"Can't remove bytes from receiver\n");
         return -1;
     }
 
@@ -2165,12 +2202,14 @@ int dlt_daemon_process_user_message_register_context(DltDaemon *daemon,
                                                      int verbose)
 {
     char local_str[DLT_DAEMON_TEXTBUFSIZE] = { '\0' };
+    int to_remove = 0;
     uint32_t len = sizeof(DltUserControlMsgRegisterContext);
     DltUserControlMsgRegisterContext userctxt;
     char description[DLT_DAEMON_DESCSIZE + 1] = { '\0' };
     DltDaemonApplication *application = NULL;
     DltDaemonContext *context = NULL;
     DltServiceGetLogInfoRequest *req = NULL;
+    char *origin;
 
     DltMessage msg;
 
@@ -2187,8 +2226,15 @@ int dlt_daemon_process_user_message_register_context(DltDaemon *daemon,
         return -1;
     }
 
-    memset(&userctxt, 0, len);
-    if (dlt_receiver_check_and_get(rec, &userctxt, len, 1) < 0)
+    memset(&userctxt, 0, sizeof(DltUserControlMsgRegisterContext));
+    origin = rec->buf;
+
+    to_remove = dlt_receiver_check_and_get(rec,
+                                           &userctxt,
+                                           len,
+                                           DLT_RCV_SKIP_HEADER);
+
+    if (to_remove < 0)
     {
         /* Not enough bytes received */
         return -1;
@@ -2196,10 +2242,35 @@ int dlt_daemon_process_user_message_register_context(DltDaemon *daemon,
 
     len = userctxt.description_length;
 
-    if ((len > DLT_DAEMON_DESCSIZE) ||
-        (dlt_receiver_check_and_get(rec, description, len, 0) < 0))
+    if (len > DLT_DAEMON_DESCSIZE)
     {
-        dlt_log(LOG_ERR, "Unable to get application description\n");
+        len = DLT_DAEMON_DESCSIZE;
+        dlt_log(LOG_WARNING, "Context description exceeds limit\n");
+    }
+
+    /* adjust buffer pointer */
+    rec->buf += to_remove + sizeof(DltUserHeader);
+
+    if (dlt_receiver_check_and_get(rec, description, len, DLT_RCV_NONE) < 0)
+    {
+        dlt_log(LOG_ERR, "Unable to get context description\n");
+        /* in case description was not readable, set dummy description */
+        strncpy(description, "Unknown", sizeof("Unknown"));
+        /* unknown len of original description, set to 0 to not remove in next
+         * step. Because message buffer is re-adjusted the corrupted description
+         * is ignored. */
+        len = 0;
+    }
+
+    /* adjust to_remove */
+    to_remove += sizeof(DltUserHeader) + len;
+    /* point to begin of message */
+    rec->buf = origin;
+
+    /* We can now remove data. */
+    if (dlt_receiver_remove(rec, to_remove) != DLT_RETURN_OK)
+    {
+        dlt_log(LOG_WARNING,"Can't remove bytes from receiver\n");
         return -1;
     }
 
@@ -2377,7 +2448,10 @@ int dlt_daemon_process_user_message_unregister_application(DltDaemon *daemon,
         return -1;
     }
 
-    if (dlt_receiver_check_and_get(rec, &userapp, len, 1) < 0)
+    if (dlt_receiver_check_and_get(rec,
+                                   &userapp,
+                                   len,
+                                   DLT_RCV_SKIP_HEADER | DLT_RCV_REMOVE) < 0)
     {
         /* Not enough bytes received */
         return -1;
@@ -2473,7 +2547,10 @@ int dlt_daemon_process_user_message_unregister_context(DltDaemon *daemon,
         return -1;
     }
 
-    if (dlt_receiver_check_and_get(rec, &userctxt, len, 1) < 0)
+    if (dlt_receiver_check_and_get(rec,
+                                   &userctxt,
+                                   len,
+                                   DLT_RCV_SKIP_HEADER | DLT_RCV_REMOVE) < 0)
     {
         /* Not enough bytes received */
         return -1;
@@ -2711,7 +2788,7 @@ int dlt_daemon_process_user_message_log_shm(DltDaemon *daemon,
 
     memset(&userheader, 0, len);
 
-    if (dlt_receiver_check_and_get(rec, &userheader, len, 0) < 0)
+    if (dlt_receiver_check_and_get(rec, &userheader, len, DLT_RCV_REMOVE) < 0)
     {
         /* Not enough bytes received */
         return -1;
@@ -2853,7 +2930,10 @@ int dlt_daemon_process_user_message_set_app_ll_ts(DltDaemon *daemon,
     }
 
     memset(&userctxt, 0, len);
-    if (dlt_receiver_check_and_get(rec, &userctxt, len, 1) < 0)
+    if (dlt_receiver_check_and_get(rec,
+                                   &userctxt,
+                                   len,
+                                   DLT_RCV_SKIP_HEADER | DLT_RCV_REMOVE) < 0)
     {
         /* Not enough bytes received */
         return -1;
@@ -2916,7 +2996,10 @@ int dlt_daemon_process_user_message_log_mode(DltDaemon *daemon,
     }
 
     memset(&userctxt, 0, len);
-    if (dlt_receiver_check_and_get(rec, &userctxt, len, 1) < 0)
+    if (dlt_receiver_check_and_get(rec,
+                                   &userctxt,
+                                   len,
+                                   DLT_RCV_SKIP_HEADER | DLT_RCV_REMOVE) < 0)
     {
         /* Not enough bytes received */
         return -1;
diff --git a/src/shared/dlt_common.c b/src/shared/dlt_common.c
index 1eee17d..8808801 100644
--- a/src/shared/dlt_common.c
+++ b/src/shared/dlt_common.c
@@ -2270,12 +2270,12 @@ DltReturnValue dlt_receiver_move_to_begin(DltReceiver *receiver)
 int dlt_receiver_check_and_get(DltReceiver *receiver,
                                void *dest,
                                unsigned int to_get,
-                               unsigned int skip_header)
+                               unsigned int flags)
 {
     unsigned int min_size = to_get;
     void *src = NULL;
 
-    if (skip_header) {
+    if (flags & DLT_RCV_SKIP_HEADER) {
         min_size += sizeof(DltUserHeader);
     }
 
@@ -2283,23 +2283,23 @@ int dlt_receiver_check_and_get(DltReceiver *receiver,
         (receiver->bytesRcvd < (int32_t)min_size) ||
         !receiver->buf ||
         !dest) {
-        return -1;
+        return DLT_RETURN_WRONG_PARAMETER;
     }
 
     src = (void *)receiver->buf;
 
-    if (skip_header) {
+    if (flags & DLT_RCV_SKIP_HEADER) {
         src += sizeof(DltUserHeader);
     }
 
-    memset(dest, 0, to_get);
-
     memcpy(dest, src, to_get);
 
-    if (dlt_receiver_remove(receiver, min_size) == -1)
-    {
-        dlt_log(LOG_WARNING,"Can't remove bytes from receiver\n");
-        return -1;
+    if (flags & DLT_RCV_REMOVE) {
+        if (dlt_receiver_remove(receiver, min_size) != DLT_RETURN_OK)
+        {
+            dlt_log(LOG_WARNING,"Can't remove bytes from receiver\n");
+            return DLT_RETURN_ERROR;
+        }
     }
 
     return to_get;