synaptics: Bound device-supplied lengths when parsing messages

The message parser trusted the device-supplied payload length without
checking it against the number of bytes actually received.

A malicious or malfunctioning reader could thus drive an out-of-bounds
read past the received buffer.

Reject a header whose declared payload would extend past the received
data (and a too-short header).

Reported by: Keith Linneman (LinnemanLabs)
This commit is contained in:
Marco Trevisan (Treviño)
2026-07-13 08:13:46 +00:00
committed by Marco Trevisan
parent ecbc2affb2
commit da497eed89
2 changed files with 24 additions and 1 deletions
+16 -1
View File
@@ -214,10 +214,18 @@ parse_get_enrolled_users_report (bmkt_msg_resp_t *msg_resp, bmkt_response_t *res
get_enroll_templates_resp->templates[n].user_id_len = extract8 (msg_resp->payload, &offset) - 2;
if(get_enroll_templates_resp->templates[n].user_id_len > BMKT_MAX_USER_ID_LEN)
return BMKT_UNRECOGNIZED_MESSAGE;
if (offset >= msg_resp->payload_len)
return BMKT_CORRUPT_MESSAGE;
get_enroll_templates_resp->templates[n].template_status = extract8 (msg_resp->payload, &offset);
if (offset >= msg_resp->payload_len)
return BMKT_CORRUPT_MESSAGE;
get_enroll_templates_resp->templates[n].finger_id = extract8 (msg_resp->payload, &offset);
for (i = 0; i < get_enroll_templates_resp->templates[n].user_id_len; i++)
get_enroll_templates_resp->templates[n].user_id[i] = extract8 (msg_resp->payload, &offset);
{
if (offset >= msg_resp->payload_len)
return BMKT_CORRUPT_MESSAGE;
get_enroll_templates_resp->templates[n].user_id[i] = extract8 (msg_resp->payload, &offset);
}
get_enroll_templates_resp->templates[n].user_id[i] = '\0';
}
@@ -267,12 +275,19 @@ bmkt_compose_message (uint8_t *cmd, int *cmd_len, uint8_t msg_id, uint8_t seq_nu
int
bmkt_parse_message_header (uint8_t *resp_buf, int resp_len, bmkt_msg_resp_t *msg_resp)
{
if (resp_len < BMKT_MESSAGE_HEADER_LEN)
return BMKT_CORRUPT_MESSAGE;
if (resp_buf[BMKT_MESSAGE_HEADER_ID_FIELD] != BMKT_MESSAGE_HEADER_ID)
return BMKT_CORRUPT_MESSAGE;
msg_resp->seq_num = resp_buf[BMKT_MESSAGE_SEQ_NUM_FIELD];
msg_resp->msg_id = resp_buf[BMKT_MESSAGE_ID_FIELD];
msg_resp->payload_len = resp_buf[BMKT_MESSAGE_PAYLOAD_LEN_FIELD];
if (msg_resp->payload_len > resp_len - BMKT_MESSAGE_PAYLOAD_FIELD)
return BMKT_CORRUPT_MESSAGE;
if (msg_resp->payload_len > 0)
msg_resp->payload = &resp_buf[BMKT_MESSAGE_PAYLOAD_FIELD];
else
+8
View File
@@ -172,6 +172,14 @@ cmd_receive_cb (FpiUsbTransfer *transfer,
* The original code did not! */
if (msg_resp.msg_id == BMKT_RSP_GENERAL_ERROR)
{
if (msg_resp.payload_len < 2 || !msg_resp.payload)
{
fp_warn ("Received General Error with short or empty payload");
fpi_ssm_mark_failed (transfer->ssm,
fpi_device_error_new (FP_DEVICE_ERROR_PROTO));
return;
}
guint16 err;
/* XXX: It is weird that this is big endian. */