From 0c341120b5276e70ec591667409d4615f42bcbfd Mon Sep 17 00:00:00 2001 From: Harsh23Kashyap <55448981+Harsh23Kashyap@users.noreply.github.com> Date: Thu, 17 Sep 2026 01:21:53 +0530 Subject: [PATCH] dv: ignore advertised routes whose cost overflows into reachability updateRib adds the local link cost to the advertised cost without checking for overflow. A neighbor advertising Cost = MaxUint64 wraps to 0 after the addition, and the route is accepted with the best possible cost instead of being treated as unreachable. Only accept advertised costs with enough headroom below CostInfinity for the local link cost; anything else is unreachable. --- dv/dv/table_algo.go | 11 ++++++--- dv/dv/table_algo_test.go | 49 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 3 deletions(-) create mode 100644 dv/dv/table_algo_test.go diff --git a/dv/dv/table_algo.go b/dv/dv/table_algo.go index a69668d5..6314e145 100644 --- a/dv/dv/table_algo.go +++ b/dv/dv/table_algo.go @@ -36,12 +36,17 @@ func (dv *Router) updateRib(ns *table.NeighborState) { dv.rib.DirtyResetNextHop(ns.Name) for _, entry := range ns.Advert.Entries { - // Use the advertised cost by default - cost := entry.Cost + localCost + // Use the advertised cost by default. Costs at or above infinity + // are unreachable; adding the local link cost must never overflow + // into a reachable value. + cost := config.CostInfinity + if entry.Cost < config.CostInfinity-localCost { + cost = entry.Cost + localCost + } // Poison reverse - try other cost if next hop is us if entry.NextHop.Name.Equal(dv.config.RouterName()) { - if entry.OtherCost < config.CostInfinity { + if entry.OtherCost < config.CostInfinity-localCost { cost = entry.OtherCost + localCost } else { cost = config.CostInfinity diff --git a/dv/dv/table_algo_test.go b/dv/dv/table_algo_test.go new file mode 100644 index 00000000..b0e86341 --- /dev/null +++ b/dv/dv/table_algo_test.go @@ -0,0 +1,49 @@ +package dv + +import ( + "math" + "testing" + + "github.com/named-data/ndnd/dv/config" + "github.com/named-data/ndnd/dv/table" + "github.com/named-data/ndnd/dv/tlv" + enc "github.com/named-data/ndnd/std/encoding" + "github.com/stretchr/testify/require" +) + +// An advertisement entry with Cost = MaxUint64 must stay unreachable. +// updateRib adds the local link cost without checking for overflow, so +// MaxUint64 + 1 wraps to 0 and the route is accepted with the best +// possible cost. +func TestUpdateRibOverflowAdvertisedCost(t *testing.T) { + cfg := config.DefaultConfig() + cfg.Network = "/ndn" + cfg.Router = "/ndn/router1" + cfg.KeyChainUri = "insecure" + require.NoError(t, cfg.Parse()) + + dv := &Router{config: cfg, rib: table.NewRib(cfg)} + + dest, err := enc.NameFromStr("/ndn/evil") + require.NoError(t, err) + nbr, err := enc.NameFromStr("/ndn/router2") + require.NoError(t, err) + + ns := &table.NeighborState{ + Name: nbr, + Advert: &tlv.Advertisement{Entries: []*tlv.AdvEntry{{ + Destination: &tlv.Destination{Name: dest}, + NextHop: &tlv.Destination{Name: nbr}, + Cost: math.MaxUint64, + }}}, + } + + dv.updateRib(ns) + + // Block the post-update goroutine before it touches uninitialised state + dv.mutex.Lock() + defer dv.mutex.Unlock() + + // The route must be unreachable, not cost 0 + require.False(t, dv.rib.Has(dest)) +}