lib, pimd, zebra: Provide some insurance against reading bad stream data

This patch does two things:

1) Ensure the decoding of stream data between pim <-> zebra is properly
decoded and we don't read beyond the end of the stream.

2) In zebra when we are freeing memory alloced ensure that we
actually have memory to delete before we do so.

Ticket: CM-27055
Signed-off-by: Satheesh Kumar K <sathk@cumulusnetworks.com>
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
This commit is contained in:
Satheesh Kumar K 2019-10-10 21:33:19 -07:00 committed by Donald Sharp
parent fa696b3727
commit 83f8a12b8e
4 changed files with 96 additions and 41 deletions

View File

@ -81,22 +81,33 @@ char *mlag_lib_msgid_to_str(enum mlag_msg_type msg_type, char *buf, size_t size)
} }
int mlag_lib_decode_mlag_hdr(struct stream *s, struct mlag_msg *msg) int mlag_lib_decode_mlag_hdr(struct stream *s, struct mlag_msg *msg,
size_t *length)
{ {
if (s == NULL || msg == NULL) #define LIB_MLAG_HDR_LENGTH 8
*length = stream_get_endp(s);
if (s == NULL || msg == NULL || *length < LIB_MLAG_HDR_LENGTH)
return -1; return -1;
*length -= LIB_MLAG_HDR_LENGTH;
STREAM_GETL(s, msg->msg_type); STREAM_GETL(s, msg->msg_type);
STREAM_GETW(s, msg->data_len); STREAM_GETW(s, msg->data_len);
STREAM_GETW(s, msg->msg_cnt); STREAM_GETW(s, msg->msg_cnt);
return 0; return 0;
stream_failure: stream_failure:
return -1; return -1;
} }
int mlag_lib_decode_mroute_add(struct stream *s, struct mlag_mroute_add *msg) #define MLAG_MROUTE_ADD_LENGTH \
(VRF_NAMSIZ + INTERFACE_NAMSIZ + 4 + 4 + 4 + 4 + 1 + 1 + 4)
int mlag_lib_decode_mroute_add(struct stream *s, struct mlag_mroute_add *msg,
size_t *length)
{ {
if (s == NULL || msg == NULL) if (s == NULL || msg == NULL || *length < MLAG_MROUTE_ADD_LENGTH)
return -1; return -1;
STREAM_GET(msg->vrf_name, s, VRF_NAMSIZ); STREAM_GET(msg->vrf_name, s, VRF_NAMSIZ);
@ -108,14 +119,18 @@ int mlag_lib_decode_mroute_add(struct stream *s, struct mlag_mroute_add *msg)
STREAM_GETC(s, msg->am_i_dual_active); STREAM_GETC(s, msg->am_i_dual_active);
STREAM_GETL(s, msg->vrf_id); STREAM_GETL(s, msg->vrf_id);
STREAM_GET(msg->intf_name, s, INTERFACE_NAMSIZ); STREAM_GET(msg->intf_name, s, INTERFACE_NAMSIZ);
return 0; return 0;
stream_failure: stream_failure:
return -1; return -1;
} }
int mlag_lib_decode_mroute_del(struct stream *s, struct mlag_mroute_del *msg) #define MLAG_MROUTE_DEL_LENGTH (VRF_NAMSIZ + INTERFACE_NAMSIZ + 4 + 4 + 4 + 4)
int mlag_lib_decode_mroute_del(struct stream *s, struct mlag_mroute_del *msg,
size_t *length)
{ {
if (s == NULL || msg == NULL) if (s == NULL || msg == NULL || *length < MLAG_MROUTE_DEL_LENGTH)
return -1; return -1;
STREAM_GET(msg->vrf_name, s, VRF_NAMSIZ); STREAM_GET(msg->vrf_name, s, VRF_NAMSIZ);
@ -124,6 +139,7 @@ int mlag_lib_decode_mroute_del(struct stream *s, struct mlag_mroute_del *msg)
STREAM_GETL(s, msg->owner_id); STREAM_GETL(s, msg->owner_id);
STREAM_GETL(s, msg->vrf_id); STREAM_GETL(s, msg->vrf_id);
STREAM_GET(msg->intf_name, s, INTERFACE_NAMSIZ); STREAM_GET(msg->intf_name, s, INTERFACE_NAMSIZ);
return 0; return 0;
stream_failure: stream_failure:
return -1; return -1;

View File

@ -125,11 +125,14 @@ struct mlag_msg {
extern char *mlag_role2str(enum mlag_role role, char *buf, size_t size); extern char *mlag_role2str(enum mlag_role role, char *buf, size_t size);
extern char *mlag_lib_msgid_to_str(enum mlag_msg_type msg_type, char *buf, extern char *mlag_lib_msgid_to_str(enum mlag_msg_type msg_type, char *buf,
size_t size); size_t size);
extern int mlag_lib_decode_mlag_hdr(struct stream *s, struct mlag_msg *msg); extern int mlag_lib_decode_mlag_hdr(struct stream *s, struct mlag_msg *msg,
size_t *length);
extern int mlag_lib_decode_mroute_add(struct stream *s, extern int mlag_lib_decode_mroute_add(struct stream *s,
struct mlag_mroute_add *msg); struct mlag_mroute_add *msg,
size_t *length);
extern int mlag_lib_decode_mroute_del(struct stream *s, extern int mlag_lib_decode_mroute_del(struct stream *s,
struct mlag_mroute_del *msg); struct mlag_mroute_del *msg,
size_t *length);
extern int mlag_lib_decode_mlag_status(struct stream *s, extern int mlag_lib_decode_mlag_status(struct stream *s,
struct mlag_status *msg); struct mlag_status *msg);
extern int mlag_lib_decode_vxlan_update(struct stream *s, extern int mlag_lib_decode_vxlan_update(struct stream *s,

View File

@ -765,8 +765,9 @@ int pim_zebra_mlag_handle_msg(struct stream *s, int len)
struct mlag_msg mlag_msg; struct mlag_msg mlag_msg;
char buf[ZLOG_FILTER_LENGTH_MAX]; char buf[ZLOG_FILTER_LENGTH_MAX];
int rc = 0; int rc = 0;
size_t length;
rc = mlag_lib_decode_mlag_hdr(s, &mlag_msg); rc = mlag_lib_decode_mlag_hdr(s, &mlag_msg, &length);
if (rc) if (rc)
return (rc); return (rc);
@ -805,7 +806,7 @@ int pim_zebra_mlag_handle_msg(struct stream *s, int len)
case MLAG_MROUTE_ADD: { case MLAG_MROUTE_ADD: {
struct mlag_mroute_add msg; struct mlag_mroute_add msg;
rc = mlag_lib_decode_mroute_add(s, &msg); rc = mlag_lib_decode_mroute_add(s, &msg, &length);
if (rc) if (rc)
return (rc); return (rc);
pim_mlag_process_mroute_add(msg); pim_mlag_process_mroute_add(msg);
@ -813,7 +814,7 @@ int pim_zebra_mlag_handle_msg(struct stream *s, int len)
case MLAG_MROUTE_DEL: { case MLAG_MROUTE_DEL: {
struct mlag_mroute_del msg; struct mlag_mroute_del msg;
rc = mlag_lib_decode_mroute_del(s, &msg); rc = mlag_lib_decode_mroute_del(s, &msg, &length);
if (rc) if (rc)
return (rc); return (rc);
pim_mlag_process_mroute_del(msg); pim_mlag_process_mroute_del(msg);
@ -823,8 +824,7 @@ int pim_zebra_mlag_handle_msg(struct stream *s, int len)
int i; int i;
for (i = 0; i < mlag_msg.msg_cnt; i++) { for (i = 0; i < mlag_msg.msg_cnt; i++) {
rc = mlag_lib_decode_mroute_add(s, &msg, &length);
rc = mlag_lib_decode_mroute_add(s, &msg);
if (rc) if (rc)
return (rc); return (rc);
pim_mlag_process_mroute_add(msg); pim_mlag_process_mroute_add(msg);
@ -835,8 +835,7 @@ int pim_zebra_mlag_handle_msg(struct stream *s, int len)
int i; int i;
for (i = 0; i < mlag_msg.msg_cnt; i++) { for (i = 0; i < mlag_msg.msg_cnt; i++) {
rc = mlag_lib_decode_mroute_del(s, &msg, &length);
rc = mlag_lib_decode_mroute_del(s, &msg);
if (rc) if (rc)
return (rc); return (rc);
pim_mlag_process_mroute_del(msg); pim_mlag_process_mroute_del(msg);

View File

@ -667,14 +667,17 @@ int zebra_mlag_protobuf_encode_client_data(struct stream *s, uint32_t *msg_type)
int n_len = 0; int n_len = 0;
int rc = 0; int rc = 0;
char buf[ZLOG_FILTER_LENGTH_MAX]; char buf[ZLOG_FILTER_LENGTH_MAX];
size_t length;
if (IS_ZEBRA_DEBUG_MLAG) if (IS_ZEBRA_DEBUG_MLAG)
zlog_debug("%s: Entering..", __func__); zlog_debug("%s: Entering..", __func__);
rc = mlag_lib_decode_mlag_hdr(s, &mlag_msg); rc = mlag_lib_decode_mlag_hdr(s, &mlag_msg, &length);
if (rc) if (rc)
return rc; return rc;
memset(tmp_buf, 0, ZEBRA_MLAG_BUF_LIMIT);
if (IS_ZEBRA_DEBUG_MLAG) if (IS_ZEBRA_DEBUG_MLAG)
zlog_debug("%s: Mlag ProtoBuf encoding of message:%s, len:%d", zlog_debug("%s: Mlag ProtoBuf encoding of message:%s, len:%d",
__func__, __func__,
@ -688,9 +691,10 @@ int zebra_mlag_protobuf_encode_client_data(struct stream *s, uint32_t *msg_type)
ZebraMlagMrouteAdd pay_load = ZEBRA_MLAG_MROUTE_ADD__INIT; ZebraMlagMrouteAdd pay_load = ZEBRA_MLAG_MROUTE_ADD__INIT;
uint32_t vrf_name_len = 0; uint32_t vrf_name_len = 0;
rc = mlag_lib_decode_mroute_add(s, &msg); rc = mlag_lib_decode_mroute_add(s, &msg, &length);
if (rc) if (rc)
return rc; return rc;
vrf_name_len = strlen(msg.vrf_name) + 1; vrf_name_len = strlen(msg.vrf_name) + 1;
pay_load.vrf_name = XMALLOC(MTYPE_MLAG_PBUF, vrf_name_len); pay_load.vrf_name = XMALLOC(MTYPE_MLAG_PBUF, vrf_name_len);
strlcpy(pay_load.vrf_name, msg.vrf_name, vrf_name_len); strlcpy(pay_load.vrf_name, msg.vrf_name, vrf_name_len);
@ -720,7 +724,7 @@ int zebra_mlag_protobuf_encode_client_data(struct stream *s, uint32_t *msg_type)
ZebraMlagMrouteDel pay_load = ZEBRA_MLAG_MROUTE_DEL__INIT; ZebraMlagMrouteDel pay_load = ZEBRA_MLAG_MROUTE_DEL__INIT;
uint32_t vrf_name_len = 0; uint32_t vrf_name_len = 0;
rc = mlag_lib_decode_mroute_del(s, &msg); rc = mlag_lib_decode_mroute_del(s, &msg, &length);
if (rc) if (rc)
return rc; return rc;
vrf_name_len = strlen(msg.vrf_name) + 1; vrf_name_len = strlen(msg.vrf_name) + 1;
@ -749,18 +753,18 @@ int zebra_mlag_protobuf_encode_client_data(struct stream *s, uint32_t *msg_type)
ZebraMlagMrouteAddBulk Bulk_msg = ZebraMlagMrouteAddBulk Bulk_msg =
ZEBRA_MLAG_MROUTE_ADD_BULK__INIT; ZEBRA_MLAG_MROUTE_ADD_BULK__INIT;
ZebraMlagMrouteAdd **pay_load = NULL; ZebraMlagMrouteAdd **pay_load = NULL;
int i;
bool cleanup = false; bool cleanup = false;
uint32_t i, actual;
Bulk_msg.n_mroute_add = mlag_msg.msg_cnt; Bulk_msg.n_mroute_add = mlag_msg.msg_cnt;
pay_load = XMALLOC(MTYPE_MLAG_PBUF, sizeof(ZebraMlagMrouteAdd *) pay_load = XMALLOC(MTYPE_MLAG_PBUF, sizeof(ZebraMlagMrouteAdd *)
* mlag_msg.msg_cnt); * mlag_msg.msg_cnt);
for (i = 0; i < mlag_msg.msg_cnt; i++) { for (i = 0, actual = 0; i < mlag_msg.msg_cnt; i++, actual++) {
uint32_t vrf_name_len = 0; uint32_t vrf_name_len = 0;
rc = mlag_lib_decode_mroute_add(s, &msg); rc = mlag_lib_decode_mroute_add(s, &msg, &length);
if (rc) { if (rc) {
cleanup = true; cleanup = true;
break; break;
@ -796,8 +800,17 @@ int zebra_mlag_protobuf_encode_client_data(struct stream *s, uint32_t *msg_type)
tmp_buf); tmp_buf);
} }
for (i = 0; i < mlag_msg.msg_cnt; i++) { for (i = 0; i < actual; i++) {
XFREE(MTYPE_MLAG_PBUF, pay_load[i]->vrf_name); /*
* The mlag_lib_decode_mroute_add can
* fail to properly decode and cause nothing
* to be allocated. Prevent a crash
*/
if (!pay_load[i])
continue;
if (pay_load[i]->vrf_name)
XFREE(MTYPE_MLAG_PBUF, pay_load[i]->vrf_name);
if (pay_load[i]->owner_id == MLAG_OWNER_INTERFACE if (pay_load[i]->owner_id == MLAG_OWNER_INTERFACE
&& pay_load[i]->intf_name) && pay_load[i]->intf_name)
XFREE(MTYPE_MLAG_PBUF, pay_load[i]->intf_name); XFREE(MTYPE_MLAG_PBUF, pay_load[i]->intf_name);
@ -812,18 +825,18 @@ int zebra_mlag_protobuf_encode_client_data(struct stream *s, uint32_t *msg_type)
ZebraMlagMrouteDelBulk Bulk_msg = ZebraMlagMrouteDelBulk Bulk_msg =
ZEBRA_MLAG_MROUTE_DEL_BULK__INIT; ZEBRA_MLAG_MROUTE_DEL_BULK__INIT;
ZebraMlagMrouteDel **pay_load = NULL; ZebraMlagMrouteDel **pay_load = NULL;
int i;
bool cleanup = false; bool cleanup = false;
uint32_t i, actual;
Bulk_msg.n_mroute_del = mlag_msg.msg_cnt; Bulk_msg.n_mroute_del = mlag_msg.msg_cnt;
pay_load = XMALLOC(MTYPE_MLAG_PBUF, sizeof(ZebraMlagMrouteDel *) pay_load = XMALLOC(MTYPE_MLAG_PBUF, sizeof(ZebraMlagMrouteDel *)
* mlag_msg.msg_cnt); * mlag_msg.msg_cnt);
for (i = 0; i < mlag_msg.msg_cnt; i++) { for (i = 0, actual = 0; i < mlag_msg.msg_cnt; i++, actual++) {
uint32_t vrf_name_len = 0; uint32_t vrf_name_len = 0;
rc = mlag_lib_decode_mroute_del(s, &msg); rc = mlag_lib_decode_mroute_del(s, &msg, &length);
if (rc) { if (rc) {
cleanup = true; cleanup = true;
break; break;
@ -858,8 +871,17 @@ int zebra_mlag_protobuf_encode_client_data(struct stream *s, uint32_t *msg_type)
tmp_buf); tmp_buf);
} }
for (i = 0; i < mlag_msg.msg_cnt; i++) { for (i = 0; i < actual; i++) {
XFREE(MTYPE_MLAG_PBUF, pay_load[i]->vrf_name); /*
* The mlag_lib_decode_mroute_add can
* fail to properly decode and cause nothing
* to be allocated. Prevent a crash
*/
if (!pay_load[i])
continue;
if (pay_load[i]->vrf_name)
XFREE(MTYPE_MLAG_PBUF, pay_load[i]->vrf_name);
if (pay_load[i]->owner_id == MLAG_OWNER_INTERFACE if (pay_load[i]->owner_id == MLAG_OWNER_INTERFACE
&& pay_load[i]->intf_name) && pay_load[i]->intf_name)
XFREE(MTYPE_MLAG_PBUF, pay_load[i]->intf_name); XFREE(MTYPE_MLAG_PBUF, pay_load[i]->intf_name);
@ -915,6 +937,15 @@ int zebra_mlag_protobuf_encode_client_data(struct stream *s, uint32_t *msg_type)
return len; return len;
} }
static void zebra_fill_protobuf_msg(struct stream *s, char *name, int len)
{
int str_len = strlen(name) + 1;
stream_put(s, name, str_len);
/* Fill the rest with Null Character for aligning */
stream_put(s, NULL, len - str_len);
}
int zebra_mlag_protobuf_decode_message(struct stream *s, uint8_t *data, int zebra_mlag_protobuf_decode_message(struct stream *s, uint8_t *data,
uint32_t len) uint32_t len)
{ {
@ -966,7 +997,8 @@ int zebra_mlag_protobuf_decode_message(struct stream *s, uint8_t *data,
/* No Batching */ /* No Batching */
stream_putw(s, MLAG_MSG_NO_BATCH); stream_putw(s, MLAG_MSG_NO_BATCH);
/* Actual Data */ /* Actual Data */
stream_put(s, msg->peerlink, INTERFACE_NAMSIZ); zebra_fill_protobuf_msg(s, msg->peerlink,
INTERFACE_NAMSIZ);
stream_putl(s, msg->my_role); stream_putl(s, msg->my_role);
stream_putl(s, msg->peer_state); stream_putl(s, msg->peer_state);
zebra_mlag_status_update__free_unpacked(msg, NULL); zebra_mlag_status_update__free_unpacked(msg, NULL);
@ -1003,7 +1035,7 @@ int zebra_mlag_protobuf_decode_message(struct stream *s, uint8_t *data,
/* No Batching */ /* No Batching */
stream_putw(s, MLAG_MSG_NO_BATCH); stream_putw(s, MLAG_MSG_NO_BATCH);
/* Actual Data */ /* Actual Data */
stream_put(s, msg->vrf_name, VRF_NAMSIZ); zebra_fill_protobuf_msg(s, msg->vrf_name, VRF_NAMSIZ);
stream_putl(s, msg->source_ip); stream_putl(s, msg->source_ip);
stream_putl(s, msg->group_ip); stream_putl(s, msg->group_ip);
@ -1013,7 +1045,8 @@ int zebra_mlag_protobuf_decode_message(struct stream *s, uint8_t *data,
stream_putc(s, msg->am_i_dual_active); stream_putc(s, msg->am_i_dual_active);
stream_putl(s, msg->vrf_id); stream_putl(s, msg->vrf_id);
if (msg->owner_id == MLAG_OWNER_INTERFACE) if (msg->owner_id == MLAG_OWNER_INTERFACE)
stream_put(s, msg->intf_name, INTERFACE_NAMSIZ); zebra_fill_protobuf_msg(s, msg->intf_name,
INTERFACE_NAMSIZ);
else else
stream_put(s, NULL, INTERFACE_NAMSIZ); stream_put(s, NULL, INTERFACE_NAMSIZ);
zebra_mlag_mroute_add__free_unpacked(msg, NULL); zebra_mlag_mroute_add__free_unpacked(msg, NULL);
@ -1032,15 +1065,15 @@ int zebra_mlag_protobuf_decode_message(struct stream *s, uint8_t *data,
/* No Batching */ /* No Batching */
stream_putw(s, MLAG_MSG_NO_BATCH); stream_putw(s, MLAG_MSG_NO_BATCH);
/* Actual Data */ /* Actual Data */
stream_put(s, msg->vrf_name, VRF_NAMSIZ); zebra_fill_protobuf_msg(s, msg->vrf_name, VRF_NAMSIZ);
stream_putl(s, msg->source_ip); stream_putl(s, msg->source_ip);
stream_putl(s, msg->group_ip); stream_putl(s, msg->group_ip);
stream_putl(s, msg->group_ip);
stream_putl(s, msg->owner_id); stream_putl(s, msg->owner_id);
stream_putl(s, msg->vrf_id); stream_putl(s, msg->vrf_id);
if (msg->owner_id == MLAG_OWNER_INTERFACE) if (msg->owner_id == MLAG_OWNER_INTERFACE)
stream_put(s, msg->intf_name, INTERFACE_NAMSIZ); zebra_fill_protobuf_msg(s, msg->intf_name,
INTERFACE_NAMSIZ);
else else
stream_put(s, NULL, INTERFACE_NAMSIZ); stream_put(s, NULL, INTERFACE_NAMSIZ);
zebra_mlag_mroute_del__free_unpacked(msg, NULL); zebra_mlag_mroute_del__free_unpacked(msg, NULL);
@ -1067,7 +1100,8 @@ int zebra_mlag_protobuf_decode_message(struct stream *s, uint8_t *data,
msg = Bulk_msg->mroute_add[i]; msg = Bulk_msg->mroute_add[i];
stream_put(s, msg->vrf_name, VRF_NAMSIZ); zebra_fill_protobuf_msg(s, msg->vrf_name,
VRF_NAMSIZ);
stream_putl(s, msg->source_ip); stream_putl(s, msg->source_ip);
stream_putl(s, msg->group_ip); stream_putl(s, msg->group_ip);
stream_putl(s, msg->cost_to_rp); stream_putl(s, msg->cost_to_rp);
@ -1076,8 +1110,9 @@ int zebra_mlag_protobuf_decode_message(struct stream *s, uint8_t *data,
stream_putc(s, msg->am_i_dual_active); stream_putc(s, msg->am_i_dual_active);
stream_putl(s, msg->vrf_id); stream_putl(s, msg->vrf_id);
if (msg->owner_id == MLAG_OWNER_INTERFACE) if (msg->owner_id == MLAG_OWNER_INTERFACE)
stream_put(s, msg->intf_name, zebra_fill_protobuf_msg(
INTERFACE_NAMSIZ); s, msg->intf_name,
INTERFACE_NAMSIZ);
else else
stream_put(s, NULL, INTERFACE_NAMSIZ); stream_put(s, NULL, INTERFACE_NAMSIZ);
} }
@ -1106,14 +1141,16 @@ int zebra_mlag_protobuf_decode_message(struct stream *s, uint8_t *data,
msg = Bulk_msg->mroute_del[i]; msg = Bulk_msg->mroute_del[i];
stream_put(s, msg->vrf_name, VRF_NAMSIZ); zebra_fill_protobuf_msg(s, msg->vrf_name,
VRF_NAMSIZ);
stream_putl(s, msg->source_ip); stream_putl(s, msg->source_ip);
stream_putl(s, msg->group_ip); stream_putl(s, msg->group_ip);
stream_putl(s, msg->owner_id); stream_putl(s, msg->owner_id);
stream_putl(s, msg->vrf_id); stream_putl(s, msg->vrf_id);
if (msg->owner_id == MLAG_OWNER_INTERFACE) if (msg->owner_id == MLAG_OWNER_INTERFACE)
stream_put(s, msg->intf_name, zebra_fill_protobuf_msg(
INTERFACE_NAMSIZ); s, msg->intf_name,
INTERFACE_NAMSIZ);
else else
stream_put(s, NULL, INTERFACE_NAMSIZ); stream_put(s, NULL, INTERFACE_NAMSIZ);
} }