GitLab

The following discussion from !232 (merged) should be addressed:

• @benzea started a discussion: (+2 comments)

So, bmkt_parse_message_header looks like this:

int
bmkt_parse_message_header (uint8_t *resp_buf, int resp_len, bmkt_msg_resp_t *msg_resp)
{
  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 > 0)
    msg_resp->payload = &resp_buf[BMKT_MESSAGE_PAYLOAD_FIELD];
  else
    msg_resp->payload = NULL;

  return BMKT_SUCCESS;
}

So, I think this is completely fine (from a security standpoint), because the buffer has a length of MAX_TRANSFER_LEN, and payload_len is restricted to 255.

But, I stumbled over it. Could you maybe add an explicit check against resp_len to verify that the packet we received is long enough?

CC: @ALin