Summary
In Registration.AddNodes (p2p/discover/topicindex/registration.go), the path that refreshes an already-known node to a newer ENR does not maintain the bucket's DistinctNetSet IP tracker. A node already in a bucket that presents a higher-Seq record with a different IP has its endpoint adopted without the 1-per-/24-per-bucket check ever running on the new IP, and without updating the tracker.
Location
attempt, ok := b.att[id]
if ok {
// There is already an attempt scheduled with this node.
// Update the record if newer.
if attempt.Node.Seq() < n.Seq() {
attempt.Node = n // <-- adopts new IP; no ips.Remove(old)/ips.Add(new)
}
continue
}The IP gate (b.ips.Add) runs only on the new-attempt path below it, so the update path skips it entirely.
Effect
The tracker drifts from reality, and the drift can be amplified to seat more than one node per /24 in a bucket:
- Node A is admitted normally via
/24-X → tracker {/24-X: 1}.
- A presents a higher-
Seq ENR with IP in /24-Y. attempt.Node is swapped; tracker stays {/24-X: 1} while A actually lives in /24-Y.
- A new node B in
/24-Y is offered. ips.Add(/24-Y) sees count 0 and succeeds → the bucket now holds A and B both in /24-Y, defeating the "one /24 can't dominate a bucket" property.
Side effect: the old /24-X entry is never released (removeAttempt later removes the node's current IP, /24-Y), so /24-X stays counted forever and blocks honest nodes from that subnet.
Severity
Low / corner case. The node must remain reachable at the new IP to keep functioning as a registrar, so this is not pure spoofing — the attacker must actually hold IP resources in the target subnets, which is exactly the cost the /24 limit is meant to impose. The practical effect is a slow erosion of the IP-diversity guarantee plus a tracker leak, not an immediate eclipse. Worth fixing for correctness rather than as an urgent security hole.
Scope
Suggested fix
On the update path, when the IP changes, keep the tracker consistent — b.ips.Remove(oldIP) then b.ips.Add(newIP); if Add fails (the new /24 is full), reject the IP change or evict the attempt rather than silently adopting it. Add a regression test: admit a node via one /24, bump its Seq to a second /24, then verify a second node in that second /24 is still rejected and that the first /24 is released.
Summary
In
Registration.AddNodes(p2p/discover/topicindex/registration.go), the path that refreshes an already-known node to a newer ENR does not maintain the bucket'sDistinctNetSetIP tracker. A node already in a bucket that presents a higher-Seqrecord with a different IP has its endpoint adopted without the 1-per-/24-per-bucket check ever running on the new IP, and without updating the tracker.Location
The IP gate (
b.ips.Add) runs only on the new-attempt path below it, so the update path skips it entirely.Effect
The tracker drifts from reality, and the drift can be amplified to seat more than one node per /24 in a bucket:
/24-X→ tracker{/24-X: 1}.SeqENR with IP in/24-Y.attempt.Nodeis swapped; tracker stays{/24-X: 1}while A actually lives in/24-Y./24-Yis offered.ips.Add(/24-Y)sees count 0 and succeeds → the bucket now holds A and B both in/24-Y, defeating the "one /24 can't dominate a bucket" property.Side effect: the old
/24-Xentry is never released (removeAttemptlater removes the node's current IP,/24-Y), so/24-Xstays counted forever and blocks honest nodes from that subnet.Severity
Low / corner case. The node must remain reachable at the new IP to keep functioning as a registrar, so this is not pure spoofing — the attacker must actually hold IP resources in the target subnets, which is exactly the cost the /24 limit is meant to impose. The practical effect is a slow erosion of the IP-diversity guarantee plus a tracker leak, not an immediate eclipse. Worth fixing for correctness rather than as an urgent security hole.
Scope
search.goAddNodes) is not affected — it has no in-place record-update path (already-present nodes are skipped viab.contains(id)), so a node's IP is never mutated mid-life there.Suggested fix
On the update path, when the IP changes, keep the tracker consistent —
b.ips.Remove(oldIP)thenb.ips.Add(newIP); ifAddfails (the new /24 is full), reject the IP change or evict the attempt rather than silently adopting it. Add a regression test: admit a node via one /24, bump itsSeqto a second /24, then verify a second node in that second /24 is still rejected and that the first /24 is released.