summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorQuentin Young <qlyoung@cumulusnetworks.com>2017-09-21 16:03:17 -0400
committerDonald Sharp <sharpd@cumulusnetworks.com>2017-09-25 12:47:45 -0400
commit33fb59c3286e53e905b112f7769afb5bb41feead (patch)
treef32137513151e8687c8e18afdd6faddd011cd1ad
parent5039c9d5787b71c1165f0167306d0f9b16732a5d (diff)
ospf6d: fix heap use after free
During the loop we save a pointer to the next route in the table in case brouter is deleted during the course of the loop iteration. However when we call ospf6_route_remove this can trigger ospf6_route_remove on other routes in the table, one of which could be pointed at by said pointer. Since ospf6_route_next locks the route that it returns, it won't actually be deleted, instead the refcount will go to 1. In the next loop iteration, nbrouter becomes brouter, and calling ospf6_route_next on this one will finally decrement the refcount to 0, resulting in a free, which causes subsequent reads on brouter to be UAF. Since the route will have OSPF6_ROUTE_WAS_REMOVED set, provided the memory was not overwritten before we got there, we'll continue on to the next one so it is unlikely this will cause a crash in production. Solution implemented is to check if we've deleted the route and continue if so. Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
-rw-r--r--ospf6d/ospf6_intra.c15
1 files changed, 14 insertions, 1 deletions
diff --git a/ospf6d/ospf6_intra.c b/ospf6d/ospf6_intra.c
index 015776a174..e4644bb09f 100644
--- a/ospf6d/ospf6_intra.c
+++ b/ospf6d/ospf6_intra.c
@@ -1555,7 +1555,20 @@ void ospf6_intra_brouter_calculation(struct ospf6_area *oa)
for (brouter = ospf6_route_head(oa->ospf6->brouter_table); brouter;
brouter = nbrouter) {
- nbrouter = ospf6_route_next(brouter);
+ /*
+ * brouter may have been "deleted" in the last loop iteration.
+ * If this is the case there is still 1 final refcount lock
+ * taken by ospf6_route_next, that will be released by the same
+ * call and result in deletion. To avoid heap UAF we must then
+ * skip processing the deleted route.
+ */
+ if (brouter->lock == 1) {
+ nbrouter = ospf6_route_next(brouter);
+ continue;
+ } else {
+ nbrouter = ospf6_route_next(brouter);
+ }
+
brouter_id = ADV_ROUTER_IN_PREFIX(&brouter->prefix);
inet_ntop(AF_INET, &brouter_id, brouter_name,
sizeof(brouter_name));