Multiple V2 control-message handlers are broken by char* fields in protocol structs (set_log_level_v2 [#863], set_trace_status_v2, unregister_context_v2, get_log_info_v2)

[SoundMatt]

Summary

Four V2 control-message handlers in src/daemon/dlt_daemon_client.c
are broken because the DltService...V2 protocol structs they use
were defined with char * pointer fields (apid, ctid, ecid)
where inline arrays or properly-assigned pointers were intended. The
handlers either:

• cast msg->databuffer directly to one of these structs, in which
  case the pointer fields are interpreted as 8 bytes of network input
  (arbitrary-pointer dereference); or
• declare a stack/calloc'd struct, leaving the pointer fields NULL,
  then call dlt_set_id_v2(struct.apid, ...) which silently returns
  early because the destination is NULL; or
• never assign the length/pointer fields at all, leaving uninitialised
  stack values to be written to the wire and used as memcpy lengths.

Reachable from any client able to send control messages to the
daemon's control socket.

Affected functions

All four are in src/daemon/dlt_daemon_client.c:

1. dlt_daemon_control_set_log_level_v2 (line ~3660)

Already filed as #863 and patched in #864. Stack-allocated
req, NULL req.apid / req.ctid never assigned, NULL deref at
line 3723 when apidlen != 0.

2. dlt_daemon_control_set_trace_status_v2 (line ~4020)

Most severe. Direct cast of msg->databuffer to
DltServiceSetLogLevelV2 *:

req = (DltServiceSetLogLevelV2 *)(msg->databuffer);
...
dlt_set_id_v2(apid, req->apid, req->apidlen);

req->apid is now whatever 8 bytes happen to be at the right offset
in the wire data — an attacker-controlled pointer. dlt_set_id_v2
later reads from req->apid. Arbitrary memory read from network
input. Daemon either crashes (random pointer rarely valid) or leaks
memory at the attacker-supplied address.

Also has the same NULL-local-apid/ctid deref pattern as #863, and
the same (int8_t) cast on req->apidlen which produces a negative
array index when apidlen > 127.

3. dlt_daemon_control_message_unregister_context_v2 (line ~2807)

Outbound (server-built) control message. Four bugs in one function:

• contextSize = (uint8_t)(... + apidlen + ... + ctidlen + DLT_ID_SIZE);
  truncates when apidlen + ctidlen ≳ 245. msg.datasize then
  underallocates, downstream memcpy overflows the heap.
• resp.apidlen and resp.ctidlen are never assigned from the
  function's apidlen / ctidlen parameters, but are written
  to the wire response — leaking uninitialised stack memory.
• resp.apid / resp.ctid are explicitly set to NULL, never
  reassigned. memcpy(buf, resp.apid, resp.apidlen) is then
  memcpy(buf, NULL, garbage) — UB.
• The intended values (the function's parameters) are never copied
  into resp, so even when the function doesn't crash the wire
  response carries wrong lengths and zero context bytes.

Triggered every time a registered application unregisters a context
(called from dlt-daemon.c:5001).

4. dlt_daemon_control_get_log_info_v2 (line ~2098)

req is calloc'd, so its char *apid and char *ctid start NULL.
The dlt_set_id_v2(req->apid, ...) calls at lines 2154/2158 are
no-ops because the destination pointer is NULL. req->apid is then
passed to dlt_daemon_context_find_v2 (line 2199) and
dlt_daemon_application_find_v2 (line 2186) while still NULL.

The OOB-read fix in #861 addressed the bounds-check problem but
did not address this deeper parsing bug. Will follow up with a
separate PR.

Root cause

The shared DltService...V2 structs (include/dlt/dlt_common.h)
declare:

char *apid;          // pointer
char *ctid;          // pointer

…where the AUTOSAR DLT V2 wire format places the bytes inline:

[apidlen:1][apid:apidlen][ctidlen:1][ctid:ctidlen]

A char * cannot be filled directly from inline wire data, so each
handler has to either copy bytes out (which the existing
dlt_set_id_v2 calls would do if the destination were a real
buffer rather than a NULL pointer) or point the field into the
message buffer. Most handlers do neither.

Suggested approach

Per-handler, parse the wire bytes incrementally into local variables
(or point req.apid / req.ctid into msg->databuffer when each
length is non-zero, as #864 does). Don't cast msg->databuffer
directly to these structs.

Alternative (more invasive): redefine the structs with inline arrays
of DLT_V2_ID_SIZE to make casting safe — but this changes the ABI
for any external user of the headers.

I will follow up with a separate Issue + PR for each affected
function. The first follow-up
(dlt_daemon_control_message_unregister_context_v2) is in flight.

Related

• daemon: fix OOB read and parse bug in dlt_daemon_control_get_log_info_v2 (closes #870) #861 — partial fix to dlt_daemon_control_get_log_info_v2 (OOB
  read only).
• daemon: fix out-of-bounds read in dlt_daemon_control_set_log_level_v2 #862 — OOB read fix in dlt_daemon_control_set_log_level_v2.
• dlt_daemon_control_set_log_level_v2 never assigns req.apid/req.ctid; silently fails or crashes the daemon #863 — function-specific issue for pkg-config: fix library directory. #1 above.
• daemon: parse apid/ctid in dlt_daemon_control_set_log_level_v2 #864 — PR fixing dlt_daemon_control_set_log_level_v2 never assigns req.apid/req.ctid; silently fails or crashes the daemon #863.
• lib: fix heap buffer overflow in dlt_with_filename_and_line_number #865 — unrelated heap overflow in dlt_with_filename_and_line_number.