From: Guus Sliepen Date: Sun, 22 Nov 2020 10:59:54 +0000 (+0100) Subject: Fix potential NULL pointer dereference. X-Git-Url: https://git.meshlink.io/?a=commitdiff_plain;h=d65d4f25019af86563cc10d00fecce82e861e0a1;p=meshlink Fix potential NULL pointer dereference. It is possible that an attempt is made to forward a request to a node that is not reachable via meta-connections. This would trigger an assert in debug builds, or cause a segmentation fault in release builds. Add checks before attempts to send a request to node->nexthop->connection. --- diff --git a/src/net_packet.c b/src/net_packet.c index 38200e5a..b32c78f7 100644 --- a/src/net_packet.c +++ b/src/net_packet.c @@ -102,7 +102,7 @@ static void send_mtu_probe_handler(event_loop_t *loop, void *data) { } if(n->mtuprobes == 31) { - if(!n->minmtu && n->status.want_udp) { + if(!n->minmtu && n->status.want_udp && n->nexthop && n->nexthop->connection) { /* Send a dummy ANS_KEY to try to update the reflexive UDP address */ send_request(mesh, n->nexthop->connection, NULL, "%d %s %s . -1 -1 -1 0", ANS_KEY, mesh->self->name, n->name); n->status.want_udp = false; @@ -188,7 +188,13 @@ static void mtu_probe_h(meshlink_handle_t *mesh, node_t *n, vpn_packet_t *packet if(!n->status.udp_confirmed) { char *address, *port; sockaddr2str(&n->address, &address, &port); - send_request(mesh, n->nexthop->connection, NULL, "%d %s %s . -1 -1 -1 0 %s %s", ANS_KEY, n->name, n->name, address, port); + + if(n->nexthop && n->nexthop->connection) { + send_request(mesh, n->nexthop->connection, NULL, "%d %s %s . -1 -1 -1 0 %s %s", ANS_KEY, n->name, n->name, address, port); + } else { + logger(mesh, MESHLINK_WARNING, "Cannot send reflexive address to %s via %s", n->name, n->nexthop ? n->nexthop->name : n->name); + } + free(address); free(port); n->status.udp_confirmed = true; @@ -400,6 +406,11 @@ bool send_sptps_data(void *handle, uint8_t type, const void *data, size_t len) { char buf[len * 4 / 3 + 5]; b64encode(data, buf, len); + if(!to->nexthop || !to->nexthop->connection) { + logger(mesh, MESHLINK_WARNING, "Unable to forward SPTPS packet to %s via %s", to->name, to->nexthop ? to->nexthop->name : to->name); + return false; + } + /* If no valid key is known yet, send the packets using ANS_KEY requests, to ensure we get to learn the reflexive UDP address. */ if(!to->status.validkey) { diff --git a/src/protocol.c b/src/protocol.c index 5d288837..39f529a2 100644 --- a/src/protocol.c +++ b/src/protocol.c @@ -67,10 +67,14 @@ bool check_id(const char *id) { detection as well */ bool send_request(meshlink_handle_t *mesh, connection_t *c, const submesh_t *s, const char *format, ...) { - assert(c); assert(format); assert(*format); + if(!c) { + logger(mesh, MESHLINK_ERROR, "Trying to send request to non-existing connection"); + return false; + } + va_list args; char request[MAXBUFSIZE]; int len; diff --git a/src/protocol_key.c b/src/protocol_key.c index 16e97eb2..2dcb5e8d 100644 --- a/src/protocol_key.c +++ b/src/protocol_key.c @@ -71,6 +71,12 @@ static bool send_initial_sptps_data(void *handle, uint8_t type, const void *data node_t *to = handle; meshlink_handle_t *mesh = to->mesh; + + if(!to->nexthop || !to->nexthop->connection) { + logger(mesh, MESHLINK_WARNING, "Cannot send SPTPS data to %s via %s", to->name, to->nexthop ? to->nexthop->name : to->name); + return false; + } + to->sptps.send_data = send_sptps_data; char buf[len * 4 / 3 + 5]; b64encode(data, buf, len); @@ -80,6 +86,12 @@ static bool send_initial_sptps_data(void *handle, uint8_t type, const void *data bool send_req_key(meshlink_handle_t *mesh, node_t *to) { if(!node_read_public_key(mesh, to)) { logger(mesh, MESHLINK_DEBUG, "No ECDSA key known for %s", to->name); + + if(!to->nexthop || !to->nexthop->connection) { + logger(mesh, MESHLINK_WARNING, "Cannot send REQ_PUBKEY to %s via %s", to->name, to->nexthop ? to->nexthop->name : to->name); + return true; + } + char *pubkey = ecdsa_get_base64_public_key(mesh->private_key); send_request(mesh, to->nexthop->connection, NULL, "%d %s %s %d %s", REQ_KEY, mesh->self->name, to->name, REQ_PUBKEY, pubkey); free(pubkey); @@ -104,14 +116,15 @@ bool send_req_key(meshlink_handle_t *mesh, node_t *to) { static bool req_key_ext_h(meshlink_handle_t *mesh, connection_t *c, const char *request, node_t *from, int reqno) { (void)c; + if(!from->nexthop || !from->nexthop->connection) { + logger(mesh, MESHLINK_WARNING, "Cannot answer REQ_KEY from %s via %s", from->name, from->nexthop ? from->nexthop->name : from->name); + return true; + } + switch(reqno) { case REQ_PUBKEY: { char *pubkey = ecdsa_get_base64_public_key(mesh->private_key); - if(!from->nexthop || !from->nexthop->connection) { - return false; - } - if(!node_read_public_key(mesh, from)) { char hiskey[MAX_STRING_SIZE]; @@ -280,7 +293,7 @@ bool req_key_h(meshlink_handle_t *mesh, connection_t *c, const char *request) { /* This should never happen. Ignore it, unless it came directly from the connected peer, in which case we disconnect. */ return from->connection != c; } else { - if(!to->status.reachable) { + if(!to->status.reachable || !to->nexthop || !to->nexthop->connection) { logger(mesh, MESHLINK_WARNING, "Got %s from %s destination %s which is not reachable", "REQ_KEY", c->name, to_name); return true; @@ -347,6 +360,11 @@ bool ans_key_h(meshlink_handle_t *mesh, connection_t *c, const char *request) { return true; } + if(!to->nexthop || !to->nexthop->connection) { + logger(mesh, MESHLINK_WARNING, "Cannot forward ANS_KEY to %s via %s", to->name, to->nexthop ? to->nexthop->name : to->name); + return false; + } + /* Append the known UDP address of the from node, if we have a confirmed one */ if(!*address && from->status.udp_confirmed && from->address.sa.sa_family != AF_UNSPEC) { char *reflexive_address, *reflexive_port; @@ -381,6 +399,10 @@ bool ans_key_h(meshlink_handle_t *mesh, connection_t *c, const char *request) { continue; } + if(!n->nexthop->connection) { + continue; + } + logger(mesh, MESHLINK_DEBUG, "Forwarding our own reflexive UDP address to %s", n->name); send_request(mesh, c, NULL, "%d %s %s . -1 -1 -1 0 %s %s", ANS_KEY, mesh->self->name, n->name, address, port); }