aboutsummaryrefslogtreecommitdiffstats
path: root/src
diff options
context:
space:
mode:
authorNeale Ranns <nranns@cisco.com>2018-07-11 09:21:06 -0700
committerNeale Ranns <nranns@cisco.com>2018-07-12 10:26:10 +0000
commit582856273c96113a460896a5ff9aa9a29dc73336 (patch)
treedea27e72aad1b1dbb5820e5571d0c87097831f47 /src
parentdaa1276fbfdb11e9c705b5ea05e83570b8163ad8 (diff)
IGMP: validate the packets length in the DP
thanks to coverity... validate that the length of the packet on wire matches the size of the header based on the number of groups and sources. drop those that don't match. Change-Id: Iab3f3a835f6a43d9c73c5d502ea5ceccdd6985b0 Signed-off-by: Neale Ranns <nranns@cisco.com>
Diffstat (limited to 'src')
-rw-r--r--src/plugins/igmp/igmp_input.c68
-rw-r--r--src/plugins/igmp/igmp_query.c4
-rw-r--r--src/plugins/igmp/igmp_report.c8
3 files changed, 55 insertions, 25 deletions
diff --git a/src/plugins/igmp/igmp_input.c b/src/plugins/igmp/igmp_input.c
index d4563bf366d..5f54a0ba223 100644
--- a/src/plugins/igmp/igmp_input.c
+++ b/src/plugins/igmp/igmp_input.c
@@ -259,8 +259,8 @@ igmp_parse_query (vlib_main_t * vm, vlib_node_runtime_t * node,
{
igmp_membership_query_v3_t *igmp;
igmp_query_args_t *args;
+ u32 bi, next, len;
vlib_buffer_t *b;
- u32 bi, next;
next = IGMP_PARSE_QUERY_NEXT_DROP;
bi = from[0];
@@ -283,20 +283,29 @@ igmp_parse_query (vlib_main_t * vm, vlib_node_runtime_t * node,
clib_memcpy (tr->packet_data, vlib_buffer_get_current (b),
sizeof (tr->packet_data));
}
+ len = igmp_membership_query_v3_length (igmp);
/*
- * copy the contents of the query, and the interface, over
- * to the main thread for processing
+ * validate that the length on the packet on the wire
+ * corresponds to the length on the calculated v3 query
+ */
+ if (vlib_buffer_length_in_chain (vm, b) == len)
+ {
+ /*
+ * copy the contents of the query, and the interface, over
+ * to the main thread for processing
+ */
+ vlib_buffer_advance (b, -sizeof (u32));
+ args = vlib_buffer_get_current (b);
+ args->sw_if_index = vnet_buffer (b)->sw_if_index[VLIB_RX];
+
+ vl_api_rpc_call_main_thread (igmp_handle_query,
+ (u8 *) args, sizeof (*args) + len);
+ }
+ /*
+ * else a packet that is reporting more or less sources
+ * than it really has, bin it
*/
- vlib_buffer_advance (b, -sizeof (u32));
- args = vlib_buffer_get_current (b);
- args->sw_if_index = vnet_buffer (b)->sw_if_index[VLIB_RX];
-
- vl_api_rpc_call_main_thread (igmp_handle_query,
- (u8 *) args,
- sizeof (*args) +
- igmp_membership_query_v3_length
- (igmp));
vlib_validate_buffer_enqueue_x1 (vm, node, next_index, to_next,
n_left_to_next, bi, next);
@@ -351,7 +360,7 @@ igmp_parse_report (vlib_main_t * vm, vlib_node_runtime_t * node,
{
igmp_membership_report_v3_t *igmp;
igmp_report_args_t *args;
- u32 bi, next;
+ u32 bi, next, len;
vlib_buffer_t *b;
next = IGMP_PARSE_REPORT_NEXT_DROP;
@@ -368,6 +377,7 @@ igmp_parse_report (vlib_main_t * vm, vlib_node_runtime_t * node,
error = IGMP_ERROR_NONE;
b->error = error_node->errors[error];
igmp = vlib_buffer_get_current (b);
+ len = igmp_membership_report_v3_length (igmp);
ASSERT (igmp->header.type == IGMP_TYPE_membership_report_v3);
@@ -382,19 +392,27 @@ igmp_parse_report (vlib_main_t * vm, vlib_node_runtime_t * node,
}
/*
- * copy the contents of the query, and the interface, over
- * to the main thread for processing
+ * validate that the length on the packet on the wire
+ * corresponds to the length on the calculated v3 query
+ */
+ if (vlib_buffer_length_in_chain (vm, b) == len)
+ {
+ /*
+ * copy the contents of the query, and the interface, over
+ * to the main thread for processing
+ */
+ vlib_buffer_advance (b, -sizeof (u32));
+ args = vlib_buffer_get_current (b);
+ args->sw_if_index = vnet_buffer (b)->sw_if_index[VLIB_RX];
+
+ vl_api_rpc_call_main_thread (igmp_handle_report,
+ (u8 *) args, sizeof (*args) + len);
+ }
+ /*
+ * else
+ * this is a packet with more groups/sources than the
+ * header reports. bin it
*/
- vlib_buffer_advance (b, -sizeof (u32));
- args = vlib_buffer_get_current (b);
- args->sw_if_index = vnet_buffer (b)->sw_if_index[VLIB_RX];
-
- vl_api_rpc_call_main_thread (igmp_handle_report,
- (u8 *) args,
- sizeof (*args) +
- igmp_membership_report_v3_length
- (igmp));
-
vlib_validate_buffer_enqueue_x1 (vm, node, next_index, to_next,
n_left_to_next, bi, next);
}
diff --git a/src/plugins/igmp/igmp_query.c b/src/plugins/igmp/igmp_query.c
index 1513023df2e..ae9a4d51e95 100644
--- a/src/plugins/igmp/igmp_query.c
+++ b/src/plugins/igmp/igmp_query.c
@@ -36,6 +36,10 @@ igmp_query_mk_source_list (const igmp_membership_query_v3_t * q)
const ip4_address_t *s;
u16 ii, n;
+ /*
+ * we validated this packet when we accepted it in the DP, so
+ * this number is safe to use
+ */
n = clib_net_to_host_u16 (q->n_src_addresses);
if (0 == n)
diff --git a/src/plugins/igmp/igmp_report.c b/src/plugins/igmp/igmp_report.c
index 328b890c15e..2caa9361b70 100644
--- a/src/plugins/igmp/igmp_report.c
+++ b/src/plugins/igmp/igmp_report.c
@@ -25,6 +25,10 @@ igmp_group_mk_source_list (const igmp_membership_group_v3_t * r)
const ip4_address_t *s;
u16 ii, n;
+ /*
+ * we validated this packet when we accepted it in the DP, so
+ * this number is safe to use
+ */
n = clib_net_to_host_u16 (r->n_src_addresses);
if (0 == n)
@@ -172,6 +176,10 @@ igmp_handle_report (const igmp_report_args_t * args)
return;
}
+ /*
+ * we validated this packet when we accepted it in the DP, so
+ * this number is safe to use
+ */
n_groups = clib_net_to_host_u16 (args->report[0].n_groups);
igmp_group = args->report[0].groups;