diff options
| author | Quentin Young <qlyoung@cumulusnetworks.com> | 2017-09-21 16:03:17 -0400 |
|---|---|---|
| committer | Donald Sharp <sharpd@cumulusnetworks.com> | 2017-09-25 12:47:45 -0400 |
| commit | 33fb59c3286e53e905b112f7769afb5bb41feead (patch) | |
| tree | f32137513151e8687c8e18afdd6faddd011cd1ad | |
| parent | 5039c9d5787b71c1165f0167306d0f9b16732a5d (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.c | 15 |
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)); |
