michaelh/zephyr
1
0
Fork 0
Code Releases Activity

mgmt/osdp: Add length checks for commands and replies

Browse source 
For all commands and replies, the buffer length needed to build or the
length of data needed to decode needs to be checked and asserted. Right now
we do this by ad-hoc if-s. Add macros that do this at a common location.

Signed-off-by: Siddharth Chandrasekaran <sidcha.dev@gmail.com>
This commit is contained in:
Siddharth Chandrasekaran 2022-06-20 22:40:44 +02:00 • committed by Carles Cufí
commit c7fec71193
2 changed files with 61 additions and 83 deletions
Download patch file Download diff file Expand all files Collapse all files

74
subsys/mgmt/osdp/src/osdp_cp.c
View file

@ -80,6 +80,12 @@ int osdp_extract_address(int *address)
return (pd_offset == CONFIG_OSDP_NUM_CONNECTED_PD) ? 0 : -1;
}
static inline void assert_buf_len(int need, int have)
{
__ASSERT(need < have, "OOM at build command: need:%d have:%d",
need, have);
}
static int cp_build_command(struct osdp_pd *pd, uint8_t *buf, int max_len)
{
struct osdp_cmd *cmd = NULL;
@ -97,45 +103,50 @@ static int cp_build_command(struct osdp_pd *pd, uint8_t *buf, int max_len)
switch (pd->cmd_id) {
case CMD_POLL:
__fallthrough;
assert_buf_len(CMD_POLL_LEN, max_len);
buf[len++] = pd->cmd_id;
ret = 0;
break;
case CMD_LSTAT:
__fallthrough;
assert_buf_len(CMD_LSTAT_LEN, max_len);
buf[len++] = pd->cmd_id;
ret = 0;
break;
case CMD_ISTAT:
__fallthrough;
assert_buf_len(CMD_ISTAT_LEN, max_len);
buf[len++] = pd->cmd_id;
ret = 0;
break;
case CMD_OSTAT:
__fallthrough;
assert_buf_len(CMD_OSTAT_LEN, max_len);
buf[len++] = pd->cmd_id;
ret = 0;
break;
case CMD_RSTAT:
assert_buf_len(CMD_RSTAT_LEN, max_len);
buf[len++] = pd->cmd_id;
ret = 0;
break;
case CMD_ID:
if (max_len < CMD_ID_LEN) {
break;
}
assert_buf_len(CMD_ID_LEN, max_len);
buf[len++] = pd->cmd_id;
buf[len++] = 0x00;
ret = 0;
break;
case CMD_CAP:
if (max_len < CMD_CAP_LEN) {
break;
}
assert_buf_len(CMD_CAP_LEN, max_len);
buf[len++] = pd->cmd_id;
buf[len++] = 0x00;
ret = 0;
break;
case CMD_DIAG:
if (max_len < CMD_DIAG_LEN) {
break;
}
assert_buf_len(CMD_DIAG_LEN, max_len);
buf[len++] = pd->cmd_id;
buf[len++] = 0x00;
ret = 0;
break;
case CMD_OUT:
if (max_len < CMD_OUT_LEN) {
break;
}
assert_buf_len(CMD_OUT_LEN, max_len);
cmd = (struct osdp_cmd *)pd->cmd_data;
buf[len++] = pd->cmd_id;
buf[len++] = cmd->output.output_no;
@ -145,9 +156,7 @@ static int cp_build_command(struct osdp_pd *pd, uint8_t *buf, int max_len)
ret = 0;
break;
case CMD_LED:
if (max_len < CMD_LED_LEN) {
break;
}
assert_buf_len(CMD_LED_LEN, max_len);
cmd = (struct osdp_cmd *)pd->cmd_data;
buf[len++] = pd->cmd_id;
buf[len++] = cmd->led.reader;
@ -169,9 +178,7 @@ static int cp_build_command(struct osdp_pd *pd, uint8_t *buf, int max_len)
ret = 0;
break;
case CMD_BUZ:
if (max_len < CMD_BUZ_LEN) {
break;
}
assert_buf_len(CMD_BUZ_LEN, max_len);
cmd = (struct osdp_cmd *)pd->cmd_data;
buf[len++] = pd->cmd_id;
buf[len++] = cmd->buzzer.reader;
@ -183,9 +190,7 @@ static int cp_build_command(struct osdp_pd *pd, uint8_t *buf, int max_len)
break;
case CMD_TEXT:
cmd = (struct osdp_cmd *)pd->cmd_data;
if (max_len < (CMD_TEXT_LEN + cmd->text.length)) {
break;
}
assert_buf_len(CMD_TEXT_LEN + cmd->text.length, max_len);
buf[len++] = pd->cmd_id;
buf[len++] = cmd->text.reader;
buf[len++] = cmd->text.control_code;
@ -199,9 +204,7 @@ static int cp_build_command(struct osdp_pd *pd, uint8_t *buf, int max_len)
ret = 0;
break;
case CMD_COMSET:
if (max_len < CMD_COMSET_LEN) {
break;
}
assert_buf_len(CMD_COMSET_LEN, max_len);
cmd = (struct osdp_cmd *)pd->cmd_data;
buf[len++] = pd->cmd_id;
buf[len++] = cmd->comset.address;
@ -217,9 +220,7 @@ static int cp_build_command(struct osdp_pd *pd, uint8_t *buf, int max_len)
LOG_ERR("Cannot perform KEYSET without SC!");
return OSDP_CP_ERR_GENERIC;
}
if (max_len < CMD_KEYSET_LEN) {
break;
}
assert_buf_len(CMD_KEYSET_LEN, max_len);
buf[len++] = pd->cmd_id;
buf[len++] = 1; /* key type (1: SCBK) */
buf[len++] = 16; /* key length in bytes */
@ -228,7 +229,8 @@ static int cp_build_command(struct osdp_pd *pd, uint8_t *buf, int max_len)
ret = 0;
break;
case CMD_CHLNG:
if (smb == NULL || max_len < CMD_CHLNG_LEN) {
assert_buf_len(CMD_CHLNG_LEN, max_len);
if (smb == NULL) {
break;
}
osdp_fill_random(pd->sc.cp_random, 8);
@ -242,7 +244,8 @@ static int cp_build_command(struct osdp_pd *pd, uint8_t *buf, int max_len)
ret = 0;
break;
case CMD_SCRYPT:
if (smb == NULL || max_len < CMD_SCRYPT_LEN) {
assert_buf_len(CMD_SCRYPT_LEN, max_len);
if (smb == NULL) {
break;
}
osdp_compute_cp_cryptogram(pd);
@ -489,6 +492,11 @@ static int cp_decode_response(struct osdp_pd *pd, uint8_t *buf, int len)
return OSDP_CP_ERR_UNKNOWN;
}
if (ret != OSDP_CP_ERR_NONE) {
LOG_ERR("Failed to decode response: REPLY(%02x) for CMD(%02x)",
pd->cmd_id, pd->reply_id);
}
if (pd->cmd_id != CMD_POLL) {
LOG_DBG("CMD(%02x) REPLY(%02x)", pd->cmd_id, pd->reply_id);
}

70
subsys/mgmt/osdp/src/osdp_pd.c
View file

@ -299,7 +299,6 @@ static int pd_decode_command(struct osdp_pd *pd, uint8_t *buf, int len)
#ifdef CONFIG_OSDP_SC_ENABLED
case CMD_KEYSET:
if (len != CMD_KEYSET_DATA_LEN) {
LOG_ERR("CMD_KEYSET length mismatch! %d/18", len);
break;
}
/**
@ -342,7 +341,6 @@ static int pd_decode_command(struct osdp_pd *pd, uint8_t *buf, int len)
break;
}
if (len != CMD_CHLNG_DATA_LEN) {
LOG_ERR("CMD_CHLNG length mismatch! %d/8", len);
break;
}
osdp_sc_init(pd);
@ -356,7 +354,6 @@ static int pd_decode_command(struct osdp_pd *pd, uint8_t *buf, int len)
}
case CMD_SCRYPT:
if (len != CMD_SCRYPT_DATA_LEN) {
LOG_ERR("CMD_SCRYPT length mismatch! %d/16", len);
break;
}
for (i = 0; i < 16; i++) {
@ -373,11 +370,12 @@ static int pd_decode_command(struct osdp_pd *pd, uint8_t *buf, int len)
return OSDP_PD_ERR_REPLY;
}
if (ret != 0) {
LOG_ERR("Invalid command structure. CMD: %02x, Len: %d",
pd->cmd_id, len);
if (ret == OSDP_PD_ERR_GENERIC) {
LOG_ERR("Failed to decode command: CMD(%02x) Len:%d ret:%d",
pd->cmd_id, len, ret);
pd->reply_id = REPLY_NAK;
pd->cmd_data[0] = OSDP_PD_NAK_CMD_LEN;
ret = OSDP_PD_ERR_REPLY;
}
if (pd->cmd_id != CMD_POLL) {
@ -387,6 +385,12 @@ static int pd_decode_command(struct osdp_pd *pd, uint8_t *buf, int len)
return ret;
}
static inline void assert_buf_len(int need, int have)
{
__ASSERT(need < have, "OOM at build command: need:%d have:%d",
need, have);
}
/**
* Returns:
* +ve: length of command
@ -404,25 +408,15 @@ static int pd_build_reply(struct osdp_pd *pd, uint8_t *buf, int max_len)
#endif
buf += data_off;
max_len -= data_off;
if (max_len <= 0) {
LOG_ERR("Out of buffer space!");
return -1;
}
switch (pd->reply_id) {
case REPLY_ACK:
if (max_len < REPLY_ACK_LEN) {
LOG_ERR("Out of buffer space!");
break;
}
assert_buf_len(REPLY_ACK_LEN, max_len);
buf[len++] = pd->reply_id;
ret = OSDP_PD_ERR_NONE;
break;
case REPLY_PDID:
if (max_len < REPLY_PDID_LEN) {
LOG_ERR("Out of buffer space!");
break;
}
assert_buf_len(REPLY_PDID_LEN, max_len);
buf[len++] = pd->reply_id;
buf[len++] = BYTE_0(pd->id.vendor_code);
@ -443,10 +437,7 @@ static int pd_build_reply(struct osdp_pd *pd, uint8_t *buf, int max_len)
ret = OSDP_PD_ERR_NONE;
break;
case REPLY_PDCAP:
if (max_len < REPLY_PDCAP_LEN) {
LOG_ERR("Out of buffer space!");
break;
}
assert_buf_len(REPLY_PDCAP_LEN, max_len);
buf[len++] = pd->reply_id;
for (i = 0; i < OSDP_PD_CAP_SENTINEL; i++) {
if (pd->cap[i].function_code != i) {
@ -464,29 +455,20 @@ static int pd_build_reply(struct osdp_pd *pd, uint8_t *buf, int max_len)
ret = OSDP_PD_ERR_NONE;
break;
case REPLY_LSTATR:
if (max_len < REPLY_LSTATR_LEN) {
LOG_ERR("Out of buffer space!");
break;
}
assert_buf_len(REPLY_LSTATR_LEN, max_len);
buf[len++] = pd->reply_id;
buf[len++] = ISSET_FLAG(pd, PD_FLAG_TAMPER);
buf[len++] = ISSET_FLAG(pd, PD_FLAG_POWER);
ret = OSDP_PD_ERR_NONE;
break;
case REPLY_RSTATR:
if (max_len < REPLY_RSTATR_LEN) {
LOG_ERR("Out of buffer space!");
break;
}
assert_buf_len(REPLY_RSTATR_LEN, max_len);
buf[len++] = pd->reply_id;
buf[len++] = ISSET_FLAG(pd, PD_FLAG_R_TAMPER);
ret = OSDP_PD_ERR_NONE;
break;
case REPLY_COM:
if (max_len < REPLY_COM_LEN) {
LOG_ERR("Out of buffer space!");
break;
}
assert_buf_len(REPLY_COM_LEN, max_len);
/**
* If COMSET succeeds, the PD must reply with the old params and
* then switch to the new params from then then on. We have the
@ -517,10 +499,7 @@ static int pd_build_reply(struct osdp_pd *pd, uint8_t *buf, int max_len)
ret = OSDP_PD_ERR_NONE;
break;
case REPLY_NAK:
if (max_len < REPLY_NAK_LEN) {
LOG_ERR("Fatal: insufficient space for sending NAK");
return -1;
}
assert_buf_len(REPLY_NAK_LEN, max_len);
buf[len++] = pd->reply_id;
buf[len++] = pd->cmd_data[0];
ret = OSDP_PD_ERR_NONE;
@ -530,10 +509,7 @@ static int pd_build_reply(struct osdp_pd *pd, uint8_t *buf, int max_len)
if (smb == NULL) {
break;
}
if (max_len < REPLY_CCRYPT_LEN) {
LOG_ERR("Out of buffer space!");
return -1;
}
assert_buf_len(REPLY_CCRYPT_LEN, max_len);
osdp_fill_random(pd->sc.pd_random, 8);
osdp_compute_session_keys(pd);
osdp_compute_pd_cryptogram(pd);
@ -556,10 +532,7 @@ static int pd_build_reply(struct osdp_pd *pd, uint8_t *buf, int max_len)
if (smb == NULL) {
break;
}
if (max_len < REPLY_RMAC_I_LEN) {
LOG_ERR("Out of buffer space!");
return -1;
}
assert_buf_len(REPLY_RMAC_I_LEN, max_len);
osdp_compute_rmac_i(pd);
buf[len++] = pd->reply_id;
for (i = 0; i < 16; i++) {
@ -595,10 +568,7 @@ static int pd_build_reply(struct osdp_pd *pd, uint8_t *buf, int max_len)
/* catch all errors and report it as a RECORD error to CP */
LOG_ERR("Failed to build REPLY(%02x); Sending NAK instead!",
pd->reply_id);
if (max_len < REPLY_NAK_LEN) {
LOG_ERR("Fatal: insufficient space for sending NAK");
return -1;
}
assert_buf_len(REPLY_NAK_LEN, max_len);
buf[0] = REPLY_NAK;
buf[1] = OSDP_PD_NAK_RECORD;
len = 2;