From 5ae403f9e657af0e9c69927d189efe33b69bde46 Mon Sep 17 00:00:00 2001 From: thorkill Date: Tue, 7 Jul 2015 21:19:26 +0200 Subject: [PATCH 1/3] Make sure we do not allocate new edge when talking to old nodes and the same edge already exists When tinc gets ADD_EDGE from older versions it will allocate new edge in protocol_edge.c:189 due to missed case in lines 149-171 where local_address is not defined. --- src/protocol_edge.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/protocol_edge.c b/src/protocol_edge.c index 0879613c..e6e27571 100644 --- a/src/protocol_edge.c +++ b/src/protocol_edge.c @@ -168,6 +168,10 @@ bool add_edge_h(connection_t *c, const char *request) { forward_request(c, request); return true; + } else { + logger(DEBUG_PROTOCOL, LOG_WARNING, "%s:%d %s -> %s - got edge we know from older version? (%d.%d)", + __FUNCTION__, __LINE__, e->from->name, e->to->name, c->protocol_major, c->protocol_minor); + return true; } } else { sockaddrfree(&local_address); From 06d4eac9ac401e27942f451239943d8aa12a64c1 Mon Sep 17 00:00:00 2001 From: thorkill Date: Tue, 7 Jul 2015 23:14:08 +0200 Subject: [PATCH 2/3] Prevent tinc from forgeting e->local_address If ADD_EDGE came from tinc version 1.0.x local_address.sa.sa_family is set to 0. If it came from tinc version 1.1.x forwarded for older verion it will be 255 - AF_UNKNOWN. --- src/protocol_edge.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/protocol_edge.c b/src/protocol_edge.c index e6e27571..7e20a427 100644 --- a/src/protocol_edge.c +++ b/src/protocol_edge.c @@ -158,8 +158,11 @@ bool add_edge_h(connection_t *c, const char *request) { // Otherwise, just ignore it. sockaddrfree(&local_address); return true; - } else if(local_address.sa.sa_family) { + } else if(local_address.sa.sa_family && local_address.sa.sa_family != AF_UNKNOWN) { // We learned a new local address for this edge. + // local_address.sa.sa_family will be 0 if we got it from older tinc versions + // local_address.sa.sa_family will be 255 (AF_UNKNOWN) if we got it from newer versions + // but for edge which does not have local_address sockaddrfree(&e->local_address); e->local_address = local_address; From 5f6613e36fd7d890db4f201592692982c5231213 Mon Sep 17 00:00:00 2001 From: thorkill Date: Wed, 8 Jul 2015 00:36:22 +0200 Subject: [PATCH 3/3] Attempt to fix the heap-use-after-free error in mst_kruskal For some reason the edges ware removed in one direction resulting in e->reverse point into invalid memory. Do not insert edge into edge_weight_tree if not needed. --- src/edge.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/edge.c b/src/edge.c index 0e35cd1e..8feeccb6 100644 --- a/src/edge.c +++ b/src/edge.c @@ -81,13 +81,21 @@ void free_edge(edge_t *e) { } void edge_add(edge_t *e) { - splay_insert(edge_weight_tree, e); - splay_insert(e->from->edge_tree, e); + if (splay_insert(e->from->edge_tree, e)) { + e->reverse = lookup_edge(e->to, e->from); - e->reverse = lookup_edge(e->to, e->from); + if(e->reverse) + e->reverse->reverse = e; - if(e->reverse) - e->reverse->reverse = e; + if (!splay_insert(edge_weight_tree, e)) + logger(DEBUG_ALWAYS, LOG_ERR, + "%s:%d: edge from: %s to: %s exists in edge_weight_tree", + __FUNCTION__, __LINE__, e->from->name, e->to->name); + } else { + logger(DEBUG_ALWAYS, LOG_ERR, + "%s:%d: edge from: %s to: %s exists in e->from->edge_tree", + __FUNCTION__, __LINE__, e->from->name, e->to->name); + } } void edge_del(edge_t *e) {