wireguard: add local variable

The current implementation of wireguard use dereference value from
pointer, but between get and dereference the value from pointer can be
occur change in pool memory, which means that this pointer can be
invalid. Since current implementation doesn't handle with invalid
pointers, segfault can occur.

The fix add a local variable to keep index of peer from pool and also
handle with null pointers from get pointer from pool.

Type: fix
Signed-off-by: Gabriel Oginski <gabrielx.oginski@intel.com>
Change-Id: Ic161ab08266e584493338c682d827ea1fd754b98

1 files changed, 53 insertions, 17 deletions

diff --git a/src/plugins/wireguard/wireguard_input.c b/src/plugins/wireguard/wireguard_input.c
index f4d9132d948..777f0ec54b3 100644
--- a/src/plugins/wireguard/wireguard_input.c
+++ b/src/plugins/wireguard/wireguard_input.c
@@ -343,9 +343,11 @@ wg_input_post_process (vlib_main_t *vm, vlib_buffer_t *b, u16 *next,
 		       bool *is_keepalive)
 {
   next[0] = WG_INPUT_NEXT_PUNT;
+  noise_keypair_t *kp;
 
-  noise_keypair_t *kp =
-    wg_get_active_keypair (&peer->remote, data->receiver_index);
+  if ((kp = wg_get_active_keypair (&peer->remote, data->receiver_index)) ==
+      NULL)
+    return -1;
 
   if (!noise_counter_recv (&kp->kp_ctr, data->counter))
     {
@@ -632,6 +634,7 @@ wg_input_inline (vlib_main_t *vm, vlib_node_runtime_t *node,
 
   bool is_keepalive = false;
   u32 *peer_idx = NULL;
+  index_t peeri = INDEX_INVALID;
 
   while (n_left_from > 0)
     {
@@ -665,9 +668,15 @@ wg_input_inline (vlib_main_t *vm, vlib_node_runtime_t *node,
 						data->receiver_index);
 	      if (PREDICT_TRUE (peer_idx != NULL))
 		{
-		  peer = wg_peer_get (*peer_idx);
+		  peeri = *peer_idx;
+		  peer = wg_peer_get (peeri);
+		  last_rec_idx = data->receiver_index;
+		}
+	      else
+		{
+		  peer = NULL;
+		  last_rec_idx = ~0;
 		}
-	      last_rec_idx = data->receiver_index;
 	    }
 
 	  if (PREDICT_FALSE (!peer_idx))
@@ -739,7 +748,7 @@ wg_input_inline (vlib_main_t *vm, vlib_node_runtime_t *node,
 	    }
 	  else if (PREDICT_FALSE (state_cr == SC_KEEP_KEY_FRESH))
 	    {
-	      wg_send_handshake_from_mt (*peer_idx, false);
+	      wg_send_handshake_from_mt (peeri, false);
 	      goto next;
 	    }
 	  else if (PREDICT_TRUE (state_cr == SC_OK))
@@ -780,7 +789,7 @@ wg_input_inline (vlib_main_t *vm, vlib_node_runtime_t *node,
 	  t->type = header_type;
 	  t->current_length = b[0]->current_length;
 	  t->is_keepalive = is_keepalive;
-	  t->peer = peer_idx ? *peer_idx : INDEX_INVALID;
+	  t->peer = peer_idx ? peeri : INDEX_INVALID;
 	}
 
     next:
@@ -827,20 +836,37 @@ wg_input_inline (vlib_main_t *vm, vlib_node_runtime_t *node,
 	{
 	  peer_idx =
 	    wg_index_table_lookup (&wmp->index_table, data->receiver_index);
-	  peer = wg_peer_get (*peer_idx);
-	  last_rec_idx = data->receiver_index;
+	  if (PREDICT_TRUE (peer_idx != NULL))
+	    {
+	      peeri = *peer_idx;
+	      peer = wg_peer_get (peeri);
+	      last_rec_idx = data->receiver_index;
+	    }
+	  else
+	    {
+	      peer = NULL;
+	      last_rec_idx = ~0;
+	    }
 	}
 
-      if (PREDICT_FALSE (wg_input_post_process (vm, b[0], data_next, peer,
-						data, &is_keepalive) < 0))
-	goto trace;
+      if (PREDICT_TRUE (peer != NULL))
+	{
+	  if (PREDICT_FALSE (wg_input_post_process (vm, b[0], data_next, peer,
+						    data, &is_keepalive) < 0))
+	    goto trace;
+	}
+      else
+	{
+	  data_next[0] = WG_INPUT_NEXT_PUNT;
+	  goto trace;
+	}
 
       if (PREDICT_FALSE (peer_idx && (last_peer_time_idx != peer_idx)))
 	{
 	  if (PREDICT_FALSE (
 		!ip46_address_is_equal (&peer->dst.addr, &out_src_ip) ||
 		peer->dst.port != out_udp_src_port))
-	    wg_peer_update_endpoint_from_mt (*peer_idx, &out_src_ip,
+	    wg_peer_update_endpoint_from_mt (peeri, &out_src_ip,
 					     out_udp_src_port);
 	  wg_timers_any_authenticated_packet_received_opt (peer, time);
 	  wg_timers_any_authenticated_packet_traversal (peer);
@@ -860,7 +886,7 @@ wg_input_inline (vlib_main_t *vm, vlib_node_runtime_t *node,
 	  t->type = header_type;
 	  t->current_length = b[0]->current_length;
 	  t->is_keepalive = is_keepalive;
-	  t->peer = peer_idx ? *peer_idx : INDEX_INVALID;
+	  t->peer = peer_idx ? peeri : INDEX_INVALID;
 	}
 
       b += 1;
@@ -922,6 +948,7 @@ wg_input_post (vlib_main_t *vm, vlib_node_runtime_t *node, vlib_frame_t *frame,
   wg_peer_t *peer = NULL;
   u32 *peer_idx = NULL;
   u32 *last_peer_time_idx = NULL;
+  index_t peeri = INDEX_INVALID;
   u32 last_rec_idx = ~0;
   f64 time = clib_time_now (&vm->clib_time) + vm->time_offset;
 
@@ -955,8 +982,17 @@ wg_input_post (vlib_main_t *vm, vlib_node_runtime_t *node, vlib_frame_t *frame,
 	  peer_idx =
 	    wg_index_table_lookup (&wmp->index_table, data->receiver_index);
 
-	  peer = wg_peer_get (*peer_idx);
-	  last_rec_idx = data->receiver_index;
+	  if (PREDICT_TRUE (peer_idx != NULL))
+	    {
+	      peeri = *peer_idx;
+	      peer = wg_peer_get (peeri);
+	      last_rec_idx = data->receiver_index;
+	    }
+	  else
+	    {
+	      peer = NULL;
+	      last_rec_idx = ~0;
+	    }
 	}
 
       if (PREDICT_TRUE (peer != NULL))
@@ -976,7 +1012,7 @@ wg_input_post (vlib_main_t *vm, vlib_node_runtime_t *node, vlib_frame_t *frame,
 	  if (PREDICT_FALSE (
 		!ip46_address_is_equal (&peer->dst.addr, &out_src_ip) ||
 		peer->dst.port != out_udp_src_port))
-	    wg_peer_update_endpoint_from_mt (*peer_idx, &out_src_ip,
+	    wg_peer_update_endpoint_from_mt (peeri, &out_src_ip,
 					     out_udp_src_port);
 	  wg_timers_any_authenticated_packet_received_opt (peer, time);
 	  wg_timers_any_authenticated_packet_traversal (peer);
@@ -995,7 +1031,7 @@ wg_input_post (vlib_main_t *vm, vlib_node_runtime_t *node, vlib_frame_t *frame,
 	  wg_input_post_trace_t *t =
 	    vlib_add_trace (vm, node, b[0], sizeof (*t));
 	  t->next = next[0];
-	  t->peer = peer_idx ? *peer_idx : INDEX_INVALID;
+	  t->peer = peer_idx ? peeri : INDEX_INVALID;
 	}
 
       b += 1;