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)) +}