Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
aki1770-del wants to merge 2 commits into
base: master
Choose a base branch
from
Open

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

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3a6bece
fix: prevent null deref and OOB read in DLT v2 control handlers (#825…
aki1770-del Apr 2, 2026
2eaebbd
fix(dlt_daemon_client): harden DLTv2 GET_LOG_INFO and SET_LOG_LEVEL h…
aki1770-del Apr 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 50 additions & 14 deletions src/daemon/dlt_daemon_client.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2105,6 +2105,12 @@ void dlt_daemon_control_get_log_info_v2(int sock,
DltMessageV2 resp;
DltDaemonContext *context = NULL;
DltDaemonApplication *application = NULL;
/* Local buffers for variable-length apid/ctid parsed from the request.
* DltServiceGetLogInfoRequestV2 uses pointer fields (char *apid, char *ctid)
* which are NULL after calloc; dlt_set_id_v2 is a no-op on NULL.
* Sized DLT_V2_ID_SIZE + 1 so a null terminator is always present (issue #6). */
char req_apid_buf[DLT_V2_ID_SIZE + 1];
char req_ctid_buf[DLT_V2_ID_SIZE + 1];

int num_applications = 0, num_contexts = 0;
uint16_t count_app_ids = 0, count_con_ids = 0;
Expand Down Expand Up @@ -2151,11 +2157,25 @@ void dlt_daemon_control_get_log_info_v2(int sock,
db_offset = db_offset + (int)sizeof(uint8_t);
memcpy(&(req->apidlen), msg->databuffer + db_offset, sizeof(uint8_t));
db_offset = db_offset + (int)sizeof(uint8_t);
dlt_set_id_v2(req->apid, (const char *)(msg->databuffer + db_offset), req->apidlen);
if ((int)db_offset + (int)req->apidlen + (int)sizeof(uint8_t) > (int)msg->datasize) {
free(req);
return;
}
/* Issue #1: req->apid is NULL after calloc; use local buffer, then point req->apid at it. */
memset(req_apid_buf, 0, sizeof(req_apid_buf));
dlt_set_id_v2(req_apid_buf, (const char *)(msg->databuffer + db_offset), req->apidlen);
req->apid = req_apid_buf;
db_offset = db_offset + (int)req->apidlen;
memcpy(&(req->ctidlen), (const char *)(msg->databuffer + db_offset), sizeof(uint8_t));
db_offset = db_offset + (int)sizeof(uint8_t);
dlt_set_id_v2(req->ctid, (const char *)(msg->databuffer + db_offset), req->ctidlen);
if ((int)db_offset + (int)req->ctidlen + (int)DLT_ID_SIZE > (int)msg->datasize) {
free(req);
return;
}
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.
/* Issue #2: req->ctid is NULL after calloc; use local buffer, then point req->ctid at it. */
memset(req_ctid_buf, 0, sizeof(req_ctid_buf));
dlt_set_id_v2(req_ctid_buf, (const char *)(msg->databuffer + db_offset), req->ctidlen);
req->ctid = req_ctid_buf;
db_offset = db_offset + (int)req->ctidlen;
memcpy((req->com), (const char *)(msg->databuffer + db_offset), DLT_ID_SIZE);

Expand Down Expand Up @@ -3664,12 +3684,17 @@ void dlt_daemon_control_set_log_level_v2(int sock,
int verbose)
{
PRINT_FUNCTION_VERBOSE(verbose);
char *apid =NULL;
char *ctid =NULL;
char *apid = NULL;
char *ctid = NULL;
/* Issue #6: sized DLT_V2_ID_SIZE + 1 so a null terminator is always present
* even when dlt_set_id_v2 fills all DLT_V2_ID_SIZE bytes. */
char apid_buf[DLT_V2_ID_SIZE + 1] = {0};
char ctid_buf[DLT_V2_ID_SIZE + 1] = {0};
DltServiceSetLogLevelV2 req = {0};
DltDaemonContext *context = NULL;
int8_t apid_length = 0;
int8_t ctid_length = 0;
/* Issue #5: uint8_t (0-255) avoids negative indices when apidlen/ctidlen >= 128. */
uint8_t apid_length = 0;
uint8_t ctid_length = 0;
int offset = 0;

if ((daemon == NULL) || (msg == NULL) || (msg->databuffer == NULL))
Expand All @@ -3682,11 +3707,24 @@ 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);
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.
dlt_set_id_v2(req.apid, (const char *)(msg->databuffer + offset), req.apidlen);
/* Issue #3: bounds check before reading variable-length apid bytes. */
if (offset + (int)req.apidlen > msg->datasize)
return;
if (req.apidlen > 0) {
dlt_set_id_v2(apid_buf, (const char *)(msg->databuffer + offset), req.apidlen);
apid = apid_buf;
}
offset = offset + req.apidlen;
memcpy(&(req.ctidlen), msg->databuffer + offset, sizeof(uint8_t));
offset = offset + (int)sizeof(uint8_t);
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.
dlt_set_id_v2(req.ctid, (const char *)(msg->databuffer + offset), req.ctidlen);
/* Issue #4: bounds check before reading variable-length ctid bytes and the
* remaining fixed fields (log_level: 1 byte, com: DLT_ID_SIZE bytes). */
if (offset + (int)req.ctidlen + (int)sizeof(uint8_t) + DLT_ID_SIZE > msg->datasize)
return;
if (req.ctidlen > 0) {
dlt_set_id_v2(ctid_buf, (const char *)(msg->databuffer + offset), req.ctidlen);
ctid = ctid_buf;
}
offset = offset + req.ctidlen;
memcpy(&(req.log_level), msg->databuffer + offset, sizeof(uint8_t));
offset = offset + (int)sizeof(uint8_t);
Expand All @@ -3695,10 +3733,8 @@ void dlt_daemon_control_set_log_level_v2(int sock,
if (daemon_local->flags.enforceContextLLAndTS)
req.log_level = (uint8_t) getStatus(req.log_level, daemon_local->flags.contextLogLevel);

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);
apid_length = (uint8_t) req.apidlen;
ctid_length = (uint8_t) 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,
Expand Down Expand Up @@ -3728,7 +3764,7 @@ void dlt_daemon_control_set_log_level_v2(int sock,
daemon_local,
1,
apid,
apid_length,
(int8_t) apid_length,
(int8_t) req.log_level,
verbose);
}
Expand All @@ -3739,7 +3775,7 @@ void dlt_daemon_control_set_log_level_v2(int sock,
daemon_local,
0,
ctid,
ctid_length,
(int8_t) ctid_length,
(int8_t) req.log_level,
verbose);
}
Expand Down