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

diff --git a/src/plugins/wireguard/wireguard_input.c b/src/plugins/wireguard/wireguard_input.c
index f4d9132..777f0ec 100644 (file)
--- 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;