Skip to content

Commit cc31ed3

Browse files
committed
daemon: fix out-of-bounds read in dlt_daemon_control_get_log_info_v2
The fixed-size precheck at the entry of this function validates only the 11-byte minimum GET_LOG_INFO V2 request (apidlen = ctidlen = 0). The function then reads two attacker-controlled uint8_t length fields (apidlen, ctidlen) from msg->databuffer and uses them to advance an offset, before passing pointers into the buffer to dlt_set_id_v2() (which reads up to apidlen / ctidlen bytes via dlt_strnlen_s) and finishing with a 4-byte memcpy of the trailing com field. A short message with non-zero length fields causes the variable-length reads to walk past the end of msg->databuffer. Add bounds checks after each length is parsed, freeing the calloc'd request and returning when the message cannot satisfy the next read. Note: the existing early-return paths inside this function already leak the calloc'd req pointer; left untouched here to keep this commit focused on the security fix.
1 parent fd1fbf8 commit cc31ed3

1 file changed

Lines changed: 20 additions & 0 deletions

File tree

src/daemon/dlt_daemon_client.c

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2151,10 +2151,30 @@ void dlt_daemon_control_get_log_info_v2(int sock,
21512151
db_offset = db_offset + (int)sizeof(uint8_t);
21522152
memcpy(&(req->apidlen), msg->databuffer + db_offset, sizeof(uint8_t));
21532153
db_offset = db_offset + (int)sizeof(uint8_t);
2154+
2155+
/* apidlen and ctidlen are attacker-controlled (uint8_t each, up to 255).
2156+
* The fixed-size precheck at the top of this function only validates
2157+
* the empty case (apidlen = ctidlen = 0); before reading the
2158+
* variable-length apid (apidlen bytes) followed by ctidlen (1 byte),
2159+
* confirm the message is large enough to contain them. Otherwise
2160+
* dlt_set_id_v2() reads past msg->databuffer. */
2161+
if ((db_offset + (int)req->apidlen + (int)sizeof(uint8_t)) > msg->datasize) {
2162+
free(req);
2163+
return;
2164+
}
2165+
21542166
dlt_set_id_v2(req->apid, (const char *)(msg->databuffer + db_offset), req->apidlen);
21552167
db_offset = db_offset + (int)req->apidlen;
21562168
memcpy(&(req->ctidlen), (const char *)(msg->databuffer + db_offset), sizeof(uint8_t));
21572169
db_offset = db_offset + (int)sizeof(uint8_t);
2170+
2171+
/* Same check for ctid (ctidlen bytes) and the trailing com field
2172+
* (DLT_ID_SIZE bytes). */
2173+
if ((db_offset + (int)req->ctidlen + DLT_ID_SIZE) > msg->datasize) {
2174+
free(req);
2175+
return;
2176+
}
2177+
21582178
dlt_set_id_v2(req->ctid, (const char *)(msg->databuffer + db_offset), req->ctidlen);
21592179
db_offset = db_offset + (int)req->ctidlen;
21602180
memcpy((req->com), (const char *)(msg->databuffer + db_offset), DLT_ID_SIZE);

0 commit comments

Comments
 (0)