From 33fb59c3286e53e905b112f7769afb5bb41feead Mon Sep 17 00:00:00 2001 From: Quentin Young Date: Thu, 21 Sep 2017 16:03:17 -0400 Subject: [PATCH] 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 --- ospf6d/ospf6_intra.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) 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)); -- 2.39.5