From f4b82f52e8b0fcc59a4c3020724022a7bc184b1a Mon Sep 17 00:00:00 2001 From: Gabriel Oginski Date: Thu, 10 Nov 2022 09:22:17 +0000 Subject: 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 Change-Id: Ic161ab08266e584493338c682d827ea1fd754b98 --- src/plugins/wireguard/wireguard_input.c | 70 +++++++++++++++++++++++++-------- 1 file 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; -- cgit 1.2.3-korg