daemon: parse incrementally in dlt_daemon_control_set_trace_status_v2

Rewrite the handler to parse the GET request field-by-field instead of
casting msg->databuffer directly to DltServiceSetLogLevelV2 *. The struct
has char *apid / char *ctid pointer fields, so a direct cast filled them
with attacker-controlled values from network input that were then
dereferenced — and the old code also hit the same NULL-deref pattern as
COVESA#863. Add bounds checks for the two attacker-controlled length fields so
the variable-length reads stay within msg->databuffer.

Copy the apid/ctid into local fixed-size buffers through the shared
dlt_set_id_v2() helper and point apid/ctid at those buffers, keeping id
initialisation inside the helper API rather than aliasing the receive
buffer (consistent with COVESA#864/COVESA#868/COVESA#861).

Related: COVESA#866.
Signed-off-by: Matt Jones <47545907+SoundMatt@users.noreply.github.com>

Author: SoundMatt

src/daemon/dlt_daemon_client.c

@@ -4027,84 +4027,128 @@ void dlt_daemon_control_set_trace_status_v2(int sock,
 
     char *apid = NULL;
     char *ctid = NULL;
-    DltServiceSetLogLevelV2 *req = NULL;
+    char apid_buf[DLT_V2_ID_SIZE] = {0};
+    char ctid_buf[DLT_V2_ID_SIZE] = {0};
     DltDaemonContext *context = NULL;
-    int8_t apid_length = 0;
-    int8_t ctid_length = 0;
+    uint32_t service_id = 0;
+    uint8_t apidlen = 0;
+    uint8_t ctidlen = 0;
+    uint8_t log_level = 0;
+    int offset = 0;
 
     if ((daemon == NULL) || (msg == NULL) || (msg->databuffer == NULL))
         return;
 
-    //TBD: Review sizeof(DltServiceSetLogLevelV2)) or fixed size
-    if (dlt_check_rcv_data_size(msg->datasize, sizeof(DltServiceSetLogLevelV2)) < 0)
+    if (dlt_check_rcv_data_size(msg->datasize, DLT_SERVICE_SET_LOG_LEVEL_FIXED_SIZE_V2) < 0)
         return;
 
-    req = (DltServiceSetLogLevelV2 *)(msg->databuffer);
+    /* Parse wire bytes incrementally instead of casting msg->databuffer to
+     * DltServiceSetLogLevelV2 *. The struct contains char *apid / char *ctid
+     * pointer fields. Under a direct cast those fields would have been
+     * filled with attacker-controlled pointer values from network input,
+     * then dereferenced by dlt_set_id_v2 — an arbitrary memory read. The
+     * previous code also fell into the same NULL-deref pattern as #863
+     * (apid / ctid NULL when used at apid[apid_length - 1] etc.).
+     * See #866 for the broader pattern. */
+    memcpy(&service_id, msg->databuffer + offset, sizeof(uint32_t));
+    offset = offset + (int)sizeof(uint32_t);
+    memcpy(&apidlen, msg->databuffer + offset, sizeof(uint8_t));
+    offset = offset + (int)sizeof(uint8_t);
 
-    if (daemon_local->flags.enforceContextLLAndTS)
-        req->log_level = (uint8_t) getStatus(req->log_level, daemon_local->flags.contextTraceStatus);
+    /* Bounds check for apid (apidlen bytes) + the upcoming ctidlen byte.
+     * The fixed-size precheck at the top only validates the empty case. */
+    if ((offset + (int)apidlen + (int)sizeof(uint8_t)) > msg->datasize)
+        return;
 
-    apid_length = (int8_t) req->apidlen;
-    dlt_set_id_v2(apid, req->apid, req->apidlen);
-    ctid_length = (int8_t) req->ctidlen;
-    dlt_set_id_v2(ctid, req->ctid, req->ctidlen);
+    /* Copy the variable-length apid into a local fixed-size buffer through the
+     * dlt_set_id_v2() helper rather than aliasing the receive buffer, keeping
+     * id initialisation inside the shared API. */
+    if (apidlen > 0) {
+        dlt_set_id_v2(apid_buf, (const char *)(msg->databuffer + offset), apidlen);
+        apid = apid_buf;
+        offset = offset + (int)apidlen;
+    }
+    memcpy(&ctidlen, msg->databuffer + offset, sizeof(uint8_t));
+    offset = offset + (int)sizeof(uint8_t);
 
-    //TBD: Review apid[apid_length - 1] == '*'
-    if ((apid_length != 0) && (apid[apid_length - 1] == '*') && (ctid == NULL)) { /*apid provided having '*' in it and ctid is null*/
+    /* Bounds check for ctid (ctidlen bytes) + log_level (1 byte) +
+     * the trailing com field (DLT_ID_SIZE bytes). */
+    if ((offset + (int)ctidlen + (int)sizeof(uint8_t) + DLT_ID_SIZE) > msg->datasize)
+        return;
+
+    if (ctidlen > 0) {
+        dlt_set_id_v2(ctid_buf, (const char *)(msg->databuffer + offset), ctidlen);
+        ctid = ctid_buf;
+        offset = offset + (int)ctidlen;
+    }
+    memcpy(&log_level, msg->databuffer + offset, sizeof(uint8_t));
+
+    if (daemon_local->flags.enforceContextLLAndTS)
+        log_level = (uint8_t) getStatus(log_level, daemon_local->flags.contextTraceStatus);
+
+    /* Use unsigned widths throughout. Casting uint8_t apidlen / ctidlen
+     * to int8_t (as the previous code did) would produce a negative value
+     * when the length exceeds 127, underflowing apid[apid_length - 1] etc.
+     * Note: downstream helpers below still take int8_t lengths; that
+     * narrowing remains a separate latent issue, out of scope here. */
+    if ((apidlen != 0) && (apid != NULL) && (apid[apidlen - 1] == '*') && (ctid == NULL)) {
+        /* apid provided having '*' in it and ctid is null */
         dlt_daemon_find_multiple_context_and_send_trace_status_v2(sock,
                                                                daemon,
                                                                daemon_local,
                                                                1,
                                                                apid,
-                                                               (int8_t) (apid_length - 1),
-                                                               (int8_t) req->log_level,
+                                                               (int8_t) (apidlen - 1),
+                                                               (int8_t) log_level,
                                                                verbose);
     }
-    else if ((ctid_length != 0) && (ctid[ctid_length - 1] == '*') && (apid == NULL)) /*ctid provided is having '*' in it and apid is null*/
-
+    else if ((ctidlen != 0) && (ctid != NULL) && (ctid[ctidlen - 1] == '*') && (apid == NULL))
     {
+        /* ctid provided is having '*' in it and apid is null */
         dlt_daemon_find_multiple_context_and_send_trace_status_v2(sock,
                                                                daemon,
                                                                daemon_local,
                                                                0,
                                                                ctid,
-                                                               (int8_t) (ctid_length - 1),
-                                                               (int8_t) req->log_level,
+                                                               (int8_t) (ctidlen - 1),
+                                                               (int8_t) log_level,
                                                                verbose);
     }
-    else if ((apid_length != 0) && (apid[apid_length - 1] != '*') && (ctid == NULL)) /*only app id case*/
+    else if ((apidlen != 0) && (apid != NULL) && (apid[apidlen - 1] != '*') && (ctid == NULL))
     {
+        /* only app id case */
         dlt_daemon_find_multiple_context_and_send_trace_status_v2(sock,
                                                                daemon,
                                                                daemon_local,
                                                                1,
                                                                apid,
-                                                               apid_length,
-                                                               (int8_t) req->log_level,
+                                                               (int8_t) apidlen,
+                                                               (int8_t) log_level,
                                                                verbose);
     }
-    else if ((ctid_length != 0) && (ctid[ctid_length - 1] != '*') && (apid == NULL)) /*only context id case*/
+    else if ((ctidlen != 0) && (ctid != NULL) && (ctid[ctidlen - 1] != '*') && (apid == NULL))
     {
+        /* only context id case */
         dlt_daemon_find_multiple_context_and_send_trace_status_v2(sock,
                                                                daemon,
                                                                daemon_local,
                                                                0,
                                                                ctid,
-                                                               ctid_length,
-                                                               (int8_t) req->log_level,
+                                                               (int8_t) ctidlen,
+                                                               (int8_t) log_level,
                                                                verbose);
     }
     else {
-        context = dlt_daemon_context_find_v2(daemon, (uint8_t)apid_length, apid, (uint8_t)ctid_length, ctid, daemon->ecuid2len, daemon->ecuid2, verbose);
+        context = dlt_daemon_context_find_v2(daemon, apidlen, apid, ctidlen, ctid, daemon->ecuid2len, daemon->ecuid2, verbose);
 
         /* Set trace status */
         if (context != 0) {
-            dlt_daemon_send_trace_status_v2(sock, daemon, daemon_local, context, (int8_t) req->log_level, verbose);
+            dlt_daemon_send_trace_status_v2(sock, daemon, daemon_local, context, (int8_t) log_level, verbose);
         }
         else {
             dlt_vlog(LOG_ERR,
                      "Could not set trace status: %d. Context [%.4s:%.4s] not found:",
-                     req->log_level,
+                     log_level,
                      apid ? apid : "<NULL>",
                      ctid ? ctid : "<NULL>");
             dlt_daemon_control_service_response_v2(sock,