Skip to content

Commit ca53e86

Browse files
Michał Narajowskicarlescufi
Michał Narajowski
authored and
carlescufi
committed
Bluetooth: Mesh: Add msg length check for Cfg and Health models
According to spec we should ignore messages with incorrect msg size. This patch adds a check to every opcode handler. Signed-off-by: Michał Narajowski <[email protected]>
1 parent b9422ea commit ca53e86

File tree

6 files changed

+215
-111
lines changed

6 files changed

+215
-111
lines changed

‎include/bluetooth/mesh/access.h

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -166,8 +166,13 @@ struct bt_mesh_model_op {
166166
/** OpCode encoded using the BT_MESH_MODEL_OP_* macros */
167167
const uint32_t opcode;
168168

169-
/** Minimum required message length */
170-
const size_t min_len;
169+
/** Message length. If the message has variable length then this value
170+
* indicates minimum message length and should be positive. Handler
171+
* function should verify precise length based on the contents of the
172+
* message. If the message has fixed length then this value should
173+
* be negative. Use BT_MESH_LEN_* macros when defining this value.
174+
*/
175+
const ssize_t len;
171176

172177
/** @brief Handler function for this opcode.
173178
*
@@ -187,6 +192,11 @@ struct bt_mesh_model_op {
187192
#define BT_MESH_MODEL_OP_2(b0, b1) (((b0) << 8) | (b1))
188193
#define BT_MESH_MODEL_OP_3(b0, cid) ((((b0) << 16) | 0xc00000) | (cid))
189194

195+
/** Macro for encoding exact message length for fixed-length messages. */
196+
#define BT_MESH_LEN_EXACT(len) (-len)
197+
/** Macro for encoding minimum message length for variable-length messages. */
198+
#define BT_MESH_LEN_MIN(len) (len)
199+
190200
/** End of the opcode list. Must always be present. */
191201
#define BT_MESH_MODEL_OP_END { 0, 0, NULL }
192202
/** Helper to define an empty opcode list. */

‎subsys/bluetooth/mesh/access.c

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -660,9 +660,13 @@ void bt_mesh_model_recv(struct bt_mesh_net_rx *rx, struct net_buf_simple *buf)
660660
continue;
661661
}
662662

663-
if (buf->len < op->min_len) {
663+
if ((op->len >= 0) && (buf->len < (size_t)op->len)) {
664664
BT_ERR("Too short message for OpCode 0x%08x", opcode);
665665
continue;
666+
} else if ((op->len < 0) && (buf->len != (size_t)(-op->len))) {
667+
BT_ERR("Invalid message size for OpCode 0x%08x",
668+
opcode);
669+
continue;
666670
}
667671

668672
/* The callback will likely parse the buffer, so

‎subsys/bluetooth/mesh/cfg_cli.c

Lines changed: 59 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -149,22 +149,7 @@ static int net_transmit_status(struct bt_mesh_model *model,
149149
struct bt_mesh_msg_ctx *ctx,
150150
struct net_buf_simple *buf)
151151
{
152-
uint8_t *status;
153-
154-
BT_DBG("net_idx 0x%04x app_idx 0x%04x src 0x%04x len %u: %s",
155-
ctx->net_idx, ctx->app_idx, ctx->addr, buf->len,
156-
bt_hex(buf->data, buf->len));
157-
158-
if (!bt_mesh_msg_ack_ctx_match(&cli->ack_ctx, OP_NET_TRANSMIT_STATUS, ctx->addr,
159-
(void **)&status)) {
160-
return -ENOENT;
161-
}
162-
163-
*status = net_buf_simple_pull_u8(buf);
164-
165-
bt_mesh_msg_ack_ctx_rx(&cli->ack_ctx);
166-
167-
return 0;
152+
return state_status_u8(model, ctx, buf, OP_NET_TRANSMIT_STATUS);
168153
}
169154

170155
struct net_key_param {
@@ -235,6 +220,11 @@ static int net_key_list(struct bt_mesh_model *model,
235220
param->keys[i++] = net_buf_simple_pull_le16(buf) & 0xfff;
236221
}
237222

223+
if (buf->len > 0) {
224+
BT_ERR("The message size for the application opcode is incorrect.");
225+
return -EMSGSIZE;
226+
}
227+
238228
*param->key_cnt = i;
239229

240230
bt_mesh_msg_ack_ctx_rx(&cli->ack_ctx);
@@ -247,11 +237,11 @@ static int node_reset_status(struct bt_mesh_model *model,
247237
struct net_buf_simple *buf)
248238
{
249239
bool *param = NULL;
250-
BT_DBG("net_idx 0x%04x app_idx 0x%04x src 0x%04x",
251-
ctx->net_idx, ctx->app_idx, ctx->addr);
240+
BT_DBG("net_idx 0x%04x app_idx 0x%04x src 0x%04x", ctx->net_idx,
241+
ctx->app_idx, ctx->addr);
252242

253-
if (!bt_mesh_msg_ack_ctx_match(&cli->ack_ctx, OP_NODE_RESET_STATUS, ctx->addr,
254-
(void **)&param)) {
243+
if (!bt_mesh_msg_ack_ctx_match(&cli->ack_ctx, OP_NODE_RESET_STATUS,
244+
ctx->addr, (void **)&param)) {
255245
return -ENOENT;
256246
}
257247

@@ -344,6 +334,11 @@ static int app_key_list(struct bt_mesh_model *model,
344334
param->keys[i++] = net_buf_simple_pull_le16(buf) & 0xfff;
345335
}
346336

337+
if (buf->len > 0U) {
338+
BT_ERR("The message size for the application opcode is incorrect.");
339+
return -EINVAL;
340+
}
341+
347342
*param->key_cnt = i;
348343
if (param->status) {
349344
*param->status = status;
@@ -374,6 +369,11 @@ static int mod_app_status(struct bt_mesh_model *model,
374369
ctx->net_idx, ctx->app_idx, ctx->addr, buf->len,
375370
bt_hex(buf->data, buf->len));
376371

372+
if ((buf->len != 7U) && (buf->len != 9U)) {
373+
BT_ERR("The message size for the application opcode is incorrect.");
374+
return -EMSGSIZE;
375+
}
376+
377377
if (!bt_mesh_msg_ack_ctx_match(&cli->ack_ctx, OP_MOD_APP_STATUS, ctx->addr,
378378
(void **)&param)) {
379379
return -ENOENT;
@@ -424,6 +424,11 @@ static int mod_member_list_handle(struct bt_mesh_msg_ctx *ctx,
424424
uint8_t status;
425425
int i;
426426

427+
if ((vnd && buf->len < 7U) || (buf->len < 5U)) {
428+
BT_ERR("The message size for the application opcode is incorrect.");
429+
return -EMSGSIZE;
430+
}
431+
427432
status = net_buf_simple_pull_u8(buf);
428433
elem_addr = net_buf_simple_pull_le16(buf);
429434
if (vnd) {
@@ -438,9 +443,9 @@ static int mod_member_list_handle(struct bt_mesh_msg_ctx *ctx,
438443
return -ENOENT;
439444
}
440445

441-
if (buf->len % 2) {
442-
BT_WARN("Model Member List invalid length");
443-
return -EINVAL;
446+
if (buf->len % 2U) {
447+
BT_ERR("Model Member List invalid length");
448+
return -EMSGSIZE;
444449
}
445450

446451
for (i = 0; i < *param->member_cnt && buf->len; i++) {
@@ -513,6 +518,11 @@ static int mod_pub_status(struct bt_mesh_model *model,
513518
ctx->net_idx, ctx->app_idx, ctx->addr, buf->len,
514519
bt_hex(buf->data, buf->len));
515520

521+
if ((buf->len != 12U) && (buf->len != 14U)) {
522+
BT_ERR("The message size for the application opcode is incorrect.");
523+
return -EINVAL;
524+
}
525+
516526
if (!bt_mesh_msg_ack_ctx_match(&cli->ack_ctx, OP_MOD_PUB_STATUS, ctx->addr,
517527
(void **)&param)) {
518528
return -ENOENT;
@@ -590,6 +600,11 @@ static int mod_sub_status(struct bt_mesh_model *model,
590600
ctx->net_idx, ctx->app_idx, ctx->addr, buf->len,
591601
bt_hex(buf->data, buf->len));
592602

603+
if ((buf->len != 7U) && (buf->len != 9U)) {
604+
BT_ERR("The message size for the application opcode is incorrect.");
605+
return -EINVAL;
606+
}
607+
593608
if (!bt_mesh_msg_ack_ctx_match(&cli->ack_ctx, OP_MOD_SUB_STATUS, ctx->addr,
594609
(void **)&param)) {
595610
return -ENOENT;
@@ -734,27 +749,27 @@ static int hb_pub_status(struct bt_mesh_model *model,
734749
}
735750

736751
const struct bt_mesh_model_op bt_mesh_cfg_cli_op[] = {
737-
{ OP_DEV_COMP_DATA_STATUS, 15, comp_data_status },
738-
{ OP_BEACON_STATUS, 1, beacon_status },
739-
{ OP_DEFAULT_TTL_STATUS, 1, ttl_status },
740-
{ OP_FRIEND_STATUS, 1, friend_status },
741-
{ OP_GATT_PROXY_STATUS, 1, gatt_proxy_status },
742-
{ OP_RELAY_STATUS, 2, relay_status },
743-
{ OP_NET_TRANSMIT_STATUS, 1, net_transmit_status },
744-
{ OP_NET_KEY_STATUS, 3, net_key_status },
745-
{ OP_NET_KEY_LIST, 0, net_key_list },
746-
{ OP_APP_KEY_STATUS, 4, app_key_status },
747-
{ OP_APP_KEY_LIST, 3, app_key_list },
748-
{ OP_MOD_APP_STATUS, 7, mod_app_status },
749-
{ OP_SIG_MOD_APP_LIST, 5, mod_app_list },
750-
{ OP_VND_MOD_APP_LIST, 7, mod_app_list_vnd },
751-
{ OP_MOD_PUB_STATUS, 12, mod_pub_status },
752-
{ OP_MOD_SUB_STATUS, 7, mod_sub_status },
753-
{ OP_MOD_SUB_LIST, 5, mod_sub_list },
754-
{ OP_MOD_SUB_LIST_VND, 7, mod_sub_list_vnd },
755-
{ OP_HEARTBEAT_SUB_STATUS, 9, hb_sub_status },
756-
{ OP_HEARTBEAT_PUB_STATUS, 10, hb_pub_status },
757-
{ OP_NODE_RESET_STATUS, 0, node_reset_status },
752+
{ OP_DEV_COMP_DATA_STATUS, BT_MESH_LEN_MIN(15), comp_data_status },
753+
{ OP_BEACON_STATUS, BT_MESH_LEN_EXACT(1), beacon_status },
754+
{ OP_DEFAULT_TTL_STATUS, BT_MESH_LEN_EXACT(1), ttl_status },
755+
{ OP_FRIEND_STATUS, BT_MESH_LEN_EXACT(1), friend_status },
756+
{ OP_GATT_PROXY_STATUS, BT_MESH_LEN_EXACT(1), gatt_proxy_status },
757+
{ OP_RELAY_STATUS, BT_MESH_LEN_EXACT(2), relay_status },
758+
{ OP_NET_TRANSMIT_STATUS, BT_MESH_LEN_EXACT(1), net_transmit_status },
759+
{ OP_NET_KEY_STATUS, BT_MESH_LEN_EXACT(3), net_key_status },
760+
{ OP_NET_KEY_LIST, BT_MESH_LEN_MIN(0), net_key_list },
761+
{ OP_APP_KEY_STATUS, BT_MESH_LEN_EXACT(4), app_key_status },
762+
{ OP_APP_KEY_LIST, BT_MESH_LEN_MIN(3), app_key_list },
763+
{ OP_MOD_APP_STATUS, BT_MESH_LEN_MIN(7), mod_app_status },
764+
{ OP_SIG_MOD_APP_LIST, BT_MESH_LEN_MIN(5), mod_app_list },
765+
{ OP_VND_MOD_APP_LIST, BT_MESH_LEN_MIN(7), mod_app_list_vnd },
766+
{ OP_MOD_PUB_STATUS, BT_MESH_LEN_MIN(12), mod_pub_status },
767+
{ OP_MOD_SUB_STATUS, BT_MESH_LEN_MIN(7), mod_sub_status },
768+
{ OP_MOD_SUB_LIST, BT_MESH_LEN_MIN(5), mod_sub_list },
769+
{ OP_MOD_SUB_LIST_VND, BT_MESH_LEN_MIN(7), mod_sub_list_vnd },
770+
{ OP_HEARTBEAT_SUB_STATUS, BT_MESH_LEN_EXACT(9), hb_sub_status },
771+
{ OP_HEARTBEAT_PUB_STATUS, BT_MESH_LEN_EXACT(10), hb_pub_status },
772+
{ OP_NODE_RESET_STATUS, BT_MESH_LEN_EXACT(0), node_reset_status },
758773
BT_MESH_MODEL_OP_END,
759774
};
760775

0 commit comments

Comments
 (0)