commit 7fca43f7184f6eb9fbe952f1eecaafcf079502f2
author Christian Franke <nobody@nowhere.ws> Tue Jun 14 20:07:03 2016 +0200
committer Paul Jakma <paul.jakma@hpe.com> Mon Oct 17 17:41:36 2016 +0100
tree 6b6fee2ba5c0877a4032eaaffee6c241ba84dca4
parent 0d8b32a5ee43ccc4a67dd87ab21a4ab553e3bf44

ospf6d: don't access nexthops out of bounds

Given that the && is evaluated lazily from left to right,
i < OSPF6_MULTI_PATH_LIMIT should be checked prior to calling
ospf6_nexthop_is_set on the array element, not the other way around.

Signed-off-by: Christian Franke <chris@opensourcerouting.org>
Signed-off-by: Christian Franke <chris@opensourcerouting.org>
Acked-by: Donald Sharp <sharpd@cumulusnetworks.com>

ospf6d/ospf6_intra.c

diff --git a/ospf6d/ospf6_intra.c b/ospf6d/ospf6_intra.c
index 3054b59..591dab3 100644
--- a/ospf6d/ospf6_intra.c
+++ b/ospf6d/ospf6_intra.c
@@ -1289,8 +1289,8 @@
         }
       else
         {
-          for (i = 0; ospf6_nexthop_is_set (&ls_entry->nexthop[i]) &&
-               i < OSPF6_MULTI_PATH_LIMIT; i++)
+          for (i = 0; i < OSPF6_MULTI_PATH_LIMIT &&
+               ospf6_nexthop_is_set (&ls_entry->nexthop[i]); i++)
             ospf6_nexthop_copy (&route->nexthop[i], &ls_entry->nexthop[i]);
         }
 

ospf6d/ospf6_route.c

diff --git a/ospf6d/ospf6_route.c b/ospf6d/ospf6_route.c
index 1c6495b..3cfbab5 100644
--- a/ospf6d/ospf6_route.c
+++ b/ospf6d/ospf6_route.c
@@ -821,8 +821,8 @@
            OSPF6_PATH_TYPE_SUBSTR (route->path.type),
            destination, nexthop, IFNAMSIZ, ifname, duration, VNL);
 
-  for (i = 1; ospf6_nexthop_is_set (&route->nexthop[i]) &&
-       i < OSPF6_MULTI_PATH_LIMIT; i++)
+  for (i = 1; i < OSPF6_MULTI_PATH_LIMIT &&
+       ospf6_nexthop_is_set (&route->nexthop[i]); i++)
     {
       /* nexthop */
       inet_ntop (AF_INET6, &route->nexthop[i].address, nexthop,
@@ -918,8 +918,8 @@
 
   /* Nexthops */
   vty_out (vty, "Nexthop:%s", VNL);
-  for (i = 0; ospf6_nexthop_is_set (&route->nexthop[i]) &&
-       i < OSPF6_MULTI_PATH_LIMIT; i++)
+  for (i = 0; i < OSPF6_MULTI_PATH_LIMIT &&
+       ospf6_nexthop_is_set (&route->nexthop[i]); i++)
     {
       /* nexthop */
       inet_ntop (AF_INET6, &route->nexthop[i].address, nexthop,

ospf6d/ospf6_spf.c

diff --git a/ospf6d/ospf6_spf.c b/ospf6d/ospf6_spf.c
index ab18d38..ebb38b6 100644
--- a/ospf6d/ospf6_spf.c
+++ b/ospf6d/ospf6_spf.c
@@ -307,8 +307,8 @@
       if (IS_OSPF6_DEBUG_SPF (PROCESS))
         zlog_debug ("  another path found, merge");
 
-      for (i = 0; ospf6_nexthop_is_set (&v->nexthop[i]) &&
-           i < OSPF6_MULTI_PATH_LIMIT; i++)
+      for (i = 0; i < OSPF6_MULTI_PATH_LIMIT &&
+           ospf6_nexthop_is_set (&v->nexthop[i]); i++)
         {
           for (j = 0; j < OSPF6_MULTI_PATH_LIMIT; j++)
             {
@@ -356,8 +356,8 @@
   route->path.options[1] = v->options[1];
   route->path.options[2] = v->options[2];
 
-  for (i = 0; ospf6_nexthop_is_set (&v->nexthop[i]) &&
-       i < OSPF6_MULTI_PATH_LIMIT; i++)
+  for (i = 0; i < OSPF6_MULTI_PATH_LIMIT &&
+       ospf6_nexthop_is_set (&v->nexthop[i]); i++)
     ospf6_nexthop_copy (&route->nexthop[i], &v->nexthop[i]);
 
   if (v->parent)
@@ -500,8 +500,8 @@
             ospf6_nexthop_calc (w, v, lsdesc);
           else
             {
-              for (i = 0; ospf6_nexthop_is_set (&v->nexthop[i]) &&
-                   i < OSPF6_MULTI_PATH_LIMIT; i++)
+              for (i = 0; i < OSPF6_MULTI_PATH_LIMIT &&
+                   ospf6_nexthop_is_set (&v->nexthop[i]); i++)
                 ospf6_nexthop_copy (&w->nexthop[i], &v->nexthop[i]);
             }