fix: prevent null deref and OOB read in DLT v2 control handlers (#825, #824) by aki1770-del · Pull Request #832 · COVESA/dlt-daemon

aki1770-del · 2026-04-02T03:18:41Z

Summary

Two bugs in the DLT v2 control message path, both reachable by a remote DLT client sending a crafted control message, causing daemon crash (DoS).

Both fixes are in src/daemon/dlt_daemon_client.c, one PR.

Bug 1 — NULL dereference in `dlt_daemon_control_set_log_level_v2` (fixes #825)

Root cause: DltServiceSetLogLevelV2.apid/ctid are char * fields zero-initialised to NULL by req = {0}. dlt_set_id_v2() guards against a NULL destination and returns immediately — the apid/ctid bytes from the message were never stored. The subsequent dlt_set_id_v2(apid, req.apid, ...) calls at lines 3699–3701 were also no-ops (both args NULL). When apidlen > 0, the condition apid[apid_length - 1] dereferenced a NULL pointer, crashing the daemon.

UBSan: runtime error: load of null pointer of type 'char' at dlt_daemon_client.c:3703.

Trigger: Any SET_LOG_LEVEL v2 message with apidlen > 0.

Fix: Introduce stack buffers apid_buf/ctid_buf[DLT_V2_ID_SIZE], parse apid/ctid directly from msg->databuffer into these buffers, and set the apid/ctid pointers conditionally (NULL preserved for "not provided"). Remove the two broken dlt_set_id_v2(NULL, ...) calls.

Bug 2 — Heap OOB read in `dlt_daemon_control_get_log_info_v2` (fixes #824)

Root cause: The fixed-size check validates only 11 bytes (DLT_SERVICE_GET_LOG_INFO_REQUEST_FIXED_SIZE_V2). req->apidlen is a uint8_t read directly from the message (attacker-controlled, 0–255). db_offset was advanced by req->apidlen at line 2155 without checking whether it remains within msg->datasize. The memcpy at line 2156 then read past the buffer end.

ASAN: heap-buffer-overflow READ at dlt_daemon_client.c:2156.

Trigger: GET_LOG_INFO v2 message with apidlen large enough to push db_offset past msg->datasize.

Fix: Add bounds checks after reading apidlen and ctidlen before advancing db_offset. On overflow, free(req) and return.

Diff summary

src/daemon/dlt_daemon_client.c | 26 ++++++++++++++++++++------
1 file changed, 20 insertions(+), 6 deletions(-)

Safety / standards

CWE-476: NULL Pointer Dereference ([REPORT] NULL Pointer Dereference via apidlen/ctidlen Handling in SET_LOG_LEVEL V2 (src/daemon/dlt_daemon_client.c) #825)
CWE-125: Out-of-Bounds Read ([REPORT] Heap-Based Out-of-Bounds Read via Unchecked GET_LOG_INFO V2 apidlen in src/daemon/dlt_daemon_client.c #824)
CWE-20: Improper Input Validation (both)
AUTOSAR SWS_DLT_01000
ISO 26262-6 §9.4.4d

Fixes #825
Fixes #824

Signed-off-by: Akihiko Komada aki1770@gmail.com

…SA#825, COVESA#824) dlt_daemon_control_set_log_level_v2 (fixes COVESA#825): DltServiceSetLogLevelV2.apid/ctid are char* fields zero-initialised to NULL. dlt_set_id_v2() guards against a NULL destination and returns immediately, so the apid/ctid bytes from the message were never stored. The subsequent dlt_set_id_v2(apid, req.apid, ...) calls at lines 3699-3701 were also no-ops (both args NULL). When apidlen > 0, the condition apid[apid_length - 1] dereferenced a NULL pointer, crashing the daemon. Fix: introduce stack buffers apid_buf/ctid_buf[DLT_V2_ID_SIZE], parse apid/ctid directly from msg->databuffer into these buffers, and set the apid/ctid pointers conditionally (NULL preserved for "not provided"). Remove the two broken dlt_set_id_v2(NULL, ...) calls. dlt_daemon_control_get_log_info_v2 (fixes COVESA#824): The fixed-size check validates only 11 bytes. req->apidlen is a uint8_t read from the message (attacker-controlled, 0-255). db_offset was advanced by req->apidlen without bounds checking, allowing it to exceed msg->datasize. The memcpy at line 2156 then read one byte past the buffer end (ASAN: heap-buffer-overflow). Fix: add bounds checks after reading apidlen and ctidlen before advancing db_offset. On overflow, free req and return. CWE-476, CWE-125, CWE-20. AUTOSAR SWS_DLT_01000, ISO 26262-6 §9.4.4d. Signed-off-by: Akihiko Komada <aki1770@gmail.com>

Copilot

Pull request overview

This PR addresses two remotely triggerable crash bugs (DoS) in the DLT v2 daemon control-message handlers by improving parsing safety for variable-length fields.

Changes:

Add payload bounds checks in dlt_daemon_control_get_log_info_v2() before advancing offsets by attacker-controlled lengths.
Rework dlt_daemon_control_set_log_level_v2() to parse APID/CTID into local buffers instead of relying on NULL-initialized pointer fields.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-02T10:09:52Z

+        free(req);
+        return;
+    }
    dlt_set_id_v2(req->apid, (const char *)(msg->databuffer + db_offset), req->apidlen);


DltServiceGetLogInfoRequestV2.apid is a char * and remains NULL after calloc(1, ...), so dlt_set_id_v2(req->apid, ...) is a no-op and the APID from the message is never captured. This makes any request with apidlen != 0 behave as if APID was missing (and dlt_daemon_application_find_v2 will treat it as invalid). Consider parsing into a local buffer (or allocating req->apid) and setting req->apid to that buffer before calling dlt_set_id_v2 / performing lookups.

Suggested change

dlt_set_id_v2(req->apid, (const char *)(msg->databuffer + db_offset), req->apidlen);

if (req->apidlen > 0U) {

req->apid = (char *)calloc(DLT_ID_SIZE + 1U, sizeof(char));

if (req->apid == NULL) {

free(req);

return;

}

dlt_set_id_v2(req->apid, (const char *)(msg->databuffer + db_offset), req->apidlen);

}

Copilot · 2026-04-02T10:09:53Z

+    if ((int)db_offset + (int)req->ctidlen + (int)DLT_ID_SIZE > (int)msg->datasize) {
+        free(req);
+        return;
+    }


Same issue as APID: DltServiceGetLogInfoRequestV2.ctid is a char * left NULL by calloc, so dlt_set_id_v2(req->ctid, ...) is a no-op and CTID is never parsed. If CTID filtering is intended, parse into a real buffer and set req->ctid accordingly before lookups.

Suggested change

}

}

/* Allocate buffer for CTID if it has not been allocated yet */

if (req->ctid == NULL) {

req->ctid = (char *)calloc((size_t)DLT_ID_SIZE + 1U, sizeof(char));

if (req->ctid == NULL) {

free(req);

return;

}

}

Copilot · 2026-04-02T10:09:53Z

@@ -3682,11 +3692,17 @@ void dlt_daemon_control_set_log_level_v2(int sock,
    offset = offset + (int)sizeof(uint32_t);
    memcpy(&(req.apidlen), msg->databuffer + offset, sizeof(uint8_t));
    offset = offset + (int)sizeof(uint8_t);


Missing bounds validation before reading variable-length APID bytes: after req.apidlen is read, dlt_set_id_v2(apid_buf, msg->databuffer + offset, req.apidlen) can read past msg->databuffer when a crafted message sets apidlen larger than the remaining payload. Add a msg->datasize check ensuring offset + req.apidlen stays within bounds (and that there is still at least 1 byte available for ctidlen) before calling dlt_set_id_v2 and advancing offset.

Suggested change

offset = offset + (int)sizeof(uint8_t);

offset = offset + (int)sizeof(uint8_t);

/* Ensure APID bytes and following CTID length byte are within message bounds */

if ((offset + req.apidlen + 1) > msg->datasize)

return;

Copilot · 2026-04-02T10:09:53Z

+    }
    offset = offset + req.apidlen;
    memcpy(&(req.ctidlen), msg->databuffer + offset, sizeof(uint8_t));
    offset = offset + (int)sizeof(uint8_t);


Missing bounds validation for CTID / trailing fields: after reading req.ctidlen, dlt_set_id_v2(ctid_buf, ...), the subsequent reads of log_level and com assume enough bytes remain. A crafted ctidlen can push offset beyond msg->datasize and cause OOB reads. Add a msg->datasize check covering offset + req.ctidlen + sizeof(log_level) + DLT_ID_SIZE (and validate before calling dlt_set_id_v2).

Suggested change

offset = offset + (int)sizeof(uint8_t);

offset = offset + (int)sizeof(uint8_t);

/* Validate that CTID, log_level and com fields fit into the received buffer */

if (dlt_check_rcv_data_size(msg->datasize,

offset + (int)req.ctidlen + (int)sizeof(uint8_t) + DLT_ID_SIZE) < 0)

return;

Copilot · 2026-04-02T10:09:53Z

    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);

    if ((apid_length != 0) && (apid[apid_length - 1] == '*') && (ctid == NULL)) { /*apid provided having '*' in it and ctid is null*/
        dlt_daemon_find_multiple_context_and_send_log_level_v2(sock,


apid_length/ctid_length are int8_t but are assigned from attacker-controlled uint8_t lengths. Values >127 will wrap negative, which can lead to negative indexing in apid[apid_length - 1] / ctid[ctid_length - 1] and to huge size_t lengths when cast in downstream calls (e.g., strncmp(..., (size_t)len)), causing OOB reads. Use an unsigned/size type for lengths (e.g., uint8_t/size_t) and validate the lengths against DLT_V2_ID_SIZE before using them for indexing/comparisons.

Copilot · 2026-04-02T10:09:54Z

+    char apid_buf[DLT_V2_ID_SIZE] = {0};
+    char ctid_buf[DLT_V2_ID_SIZE] = {0};


apid_buf/ctid_buf are sized DLT_V2_ID_SIZE and dlt_set_id_v2 may copy up to DLT_V2_ID_SIZE bytes without guaranteeing a trailing NUL when the input has no terminator. This becomes risky because these buffers are later treated as C-strings in this function (e.g., logged via %s on the context-not-found path), which can read past the stack buffer. Consider allocating buffers as DLT_V2_ID_SIZE + 1 and ensuring explicit NUL-termination (or use length-limited formatting such as precision specifiers) whenever printing/handling these IDs as strings.

…andlers Address 6 issues flagged in PR COVESA#832 code review: GET_LOG_INFO (dlt_daemon_control_get_log_info_v2): - COVESA#1/COVESA#2: req->apid and req->ctid are NULL after calloc; dlt_set_id_v2 is a no-op on NULL, so APID/CTID from the request were silently discarded. Use local stack buffers (req_apid_buf/req_ctid_buf) and point the struct fields at them after population — mirrors the SET_LOG_LEVEL pattern. - COVESA#6 (partial): local buffers sized DLT_V2_ID_SIZE + 1 so a NUL terminator is always present even when dlt_set_id_v2 fills all DLT_V2_ID_SIZE bytes. SET_LOG_LEVEL (dlt_daemon_control_set_log_level_v2): - COVESA#3: add bounds check (offset + apidlen <= datasize) before reading variable-length APID bytes. dlt_check_rcv_data_size only validates the fixed-size minimum; crafted apidlen could read past msg->databuffer. - COVESA#4: add bounds check (offset + ctidlen + 1 + DLT_ID_SIZE <= datasize) before reading variable-length CTID bytes and the remaining fixed fields (log_level, com). Same attack surface as COVESA#3. - COVESA#5: apid_length/ctid_length changed from int8_t to uint8_t. The previous (int8_t) cast truncated values 128-255 to negative, turning apid[apid_length - 1] into a large negative-indexed OOB read. - COVESA#6: apid_buf/ctid_buf sized DLT_V2_ID_SIZE + 1 (was DLT_V2_ID_SIZE) to guarantee a NUL terminator when used as C-strings in log paths. All fixes are follow-on hardening for the DLTv2 bring-up in COVESA#830/COVESA#801. Build: no new warnings or errors. Co-Authored-By: Claude and aki1770-del <aki1770@gmail.com>

aki1770-del · 2026-04-04T14:47:39Z

Update (2026-04-04): Pushed commit 2eaebbd to address 6 additional issues flagged in automated review — all follow-on hardening for the DLTv2 bring-up in #830/#801:

pkg-config: fix library directory. #1/- Added missing build steps to INSTALL #2: req->apid/req->ctid in dlt_daemon_control_get_log_info_v2 were NULL after calloc; dlt_set_id_v2 no-ops on NULL, so APID/CTID from the request were silently discarded. Fixed with local stack buffers (mirrors the SET_LOG_LEVEL pattern already in the code).
daemon: check payload length before cast to struct #3/Remove duplicate README #4: Missing bounds checks before reading variable-length APID/CTID bytes in dlt_daemon_control_set_log_level_v2. The dlt_check_rcv_data_size guard only validates the fixed-size minimum; crafted apidlen/ctidlen could read past msg->databuffer. Two targeted checks added.
Daemon connection handling fixes #5: apid_length/ctid_length typed int8_t — values 128–255 cast to negative, turning apid[apid_length - 1] into an OOB index. Changed to uint8_t.
CMake Option: Trigger segmentation fault in case of FATAL log #6: apid_buf/ctid_buf sized DLT_V2_ID_SIZE without guaranteed NUL terminator. Sized DLT_V2_ID_SIZE + 1 in both functions.

Build clean, no new warnings.

AI-assisted — authored with Claude, reviewed by Komada.

minminlittleshrimp requested a review from Copilot April 2, 2026 10:03

Copilot started reviewing on behalf of minminlittleshrimp April 2, 2026 10:04 View session

Copilot AI reviewed Apr 2, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: prevent null deref and OOB read in DLT v2 control handlers (#825, #824)#832

fix: prevent null deref and OOB read in DLT v2 control handlers (#825, #824)#832
aki1770-del wants to merge 2 commits into
COVESA:masterfrom
aki1770-del:fix/dlt-v2-control-null-deref-oob-825-824

aki1770-del commented Apr 2, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 2, 2026

Uh oh!

Copilot AI Apr 2, 2026

Uh oh!

Copilot AI Apr 2, 2026

Uh oh!

Copilot AI Apr 2, 2026

Uh oh!

Copilot AI Apr 2, 2026

Uh oh!

Copilot AI Apr 2, 2026

Uh oh!

aki1770-del commented Apr 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-    }
+    }
+    /* Allocate buffer for CTID if it has not been allocated yet */
+    if (req->ctid == NULL) {
+        req->ctid = (char *)calloc((size_t)DLT_ID_SIZE + 1U, sizeof(char));
+        if (req->ctid == NULL) {
+            free(req);
+            return;
+        }
+    }

-    offset = offset + (int)sizeof(uint8_t);
+    offset = offset + (int)sizeof(uint8_t);
+    /* Validate that CTID, log_level and com fields fit into the received buffer */
+    if (dlt_check_rcv_data_size(msg->datasize,
+                                offset + (int)req.ctidlen + (int)sizeof(uint8_t) + DLT_ID_SIZE) < 0)
+        return;

		char apid_buf[DLT_V2_ID_SIZE] = {0};
		char ctid_buf[DLT_V2_ID_SIZE] = {0};

Conversation

aki1770-del commented Apr 2, 2026

Summary

Bug 1 — NULL dereference in dlt_daemon_control_set_log_level_v2 (fixes #825)

Bug 2 — Heap OOB read in dlt_daemon_control_get_log_info_v2 (fixes #824)

Diff summary

Safety / standards

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

aki1770-del commented Apr 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Bug 1 — NULL dereference in `dlt_daemon_control_set_log_level_v2` (fixes #825)

Bug 2 — Heap OOB read in `dlt_daemon_control_get_log_info_v2` (fixes #824)