Filter: fix undefined memory access in nexthop handling

If nexthop is special (blackhole, unreachable, prohibit), directly
asking for anything else from there accessed undefined memory. This
manifested with checking for interface name where it actually
dereferenced a pointer from there, causing a crash.

In some cases, reading ifindex might have also sometimes caused a crash.
The other data (weight, gw, gw_mpls, onlink) is stored directly inside
the nexthop EA, and even though it's shorter, in most cases it "just"
returned garbage. The only exception would be probably if the
unreachable nexthop EA was allocated right at the end of tmp_pool or
locally on stack.

Fixing this by properly checking for reachability before reading the
nexthop data.

Reported-By: Lars Gierth <larsg@systemli.org>
Reproduced-By: David Petera <david.petera@nic.cz>
Fixes: #313

diff --git a/filter/f-inst.c b/filter/f-inst.c
index ec372e93d8f16ae40237b6dca53442676e5f7471..b57b92750b9965978dc2afe8a1cc2b571469ea7c 100644 (file)
--- a/filter/f-inst.c
+++ b/filter/f-inst.c
@@ -699,9 +699,10 @@
       case SA_PROTO:   RESULT(sa.type, s, rte->src->owner->name); break;
       default:
        {
+         /* Next hop examination */
          struct eattr *nhea = ea_find(rte->attrs, &ea_gen_nexthop);
          struct nexthop_adata *nhad = nhea ? (struct nexthop_adata *) nhea->u.ptr : NULL;
-         struct nexthop *nh = nhad ? &nhad->nh : NULL;
+         struct nexthop *nh = (nhad && NEXTHOP_IS_REACHABLE(nhad)) ? &nhad->nh : NULL;
 
          switch (sa.sa_code)
          {

diff --git a/filter/test.conf b/filter/test.conf
index 9acb5d7239133d40c702dba362eacf3c1deb522f..64d130645ed06b3f36227bdfea5194bb77c69480 100644 (file)
--- a/filter/test.conf
+++ b/filter/test.conf
@@ -2891,11 +2891,22 @@ protocol static
 }
 
 # Regression test for crash on setting link-local gw without interface (#257)
+# and reading undefined nexthop parameters (#313)
 protocol static {
-  route 2001:db8::/42 unreachable;
-  ipv6 {
-    import filter {
-      gw = fe80::f00:ba1;
-    };
-  };
+       route 3fff:0:1::/48 unreachable;
+       route 3fff:0:2::/49 blackhole;
+       route 3fff:0:3::/50 prohibit;
+       route 3fff:0:4::/51 prohibit;
+       ipv6 {
+               import filter {
+                       print net, "|", dest, "|", gw, "|", ifname, "|", ifindex, "|", weight, "|", gw_mpls, "|", onlink;
+                       if net.len = 48 then
+                               gw = fe80::f00:ba1;
+                       else if net.len = 49 then
+                               onlink = true;
+                       else if net.len = 50 then
+                               ifname = "lo";
+                       reject;
+               };
+       };
 }