]> git.proxmox.com Git - mirror_frr.git/commitdiff
ospf6d: fix heap use after free
authorQuentin Young <qlyoung@cumulusnetworks.com>
Thu, 21 Sep 2017 20:03:17 +0000 (16:03 -0400)
committerQuentin Young <qlyoung@cumulusnetworks.com>
Fri, 22 Sep 2017 08:56:25 +0000 (04:56 -0400)
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>
ospf6d/ospf6_intra.c

index dc9fd79c1ff51e3380193b3813919454ad1fd199..a19c3f40763f6797993f2db33eeb483566115804 100644 (file)
@@ -1564,7 +1564,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));