# #37352 \[BC-Insight] Missing Liveness Check in \`collectTableNodes()\`

**Submitted on Dec 2nd 2024 at 17:42:35 UTC by @CertiK for** [**Attackathon | Ethereum Protocol**](https://immunefi.com/audit-competition/ethereum-protocol-attackathon)

* **Report ID:** #37352
* **Report Type:** Blockchain/DLT
* **Report severity:** Insight
* **Target:** <https://github.com/ledgerwatch/erigon>
* **Impacts:**
  * (Specifications) A bug in specifications with no direct impact on client implementations

## Description

## Brief/Intro

An issue that the node misses liveness check will be added to the node table with `collectTableNodes()` was identified in the Ethereum client Erigon ( <https://github.com/erigontech/erigon> ).

## Vulnerability Details

Affected Codebase:\
<https://github.com/erigontech/erigon/tree/v2.61.0-beta1>

The function `collectTableNodes()` is intended to collect all the nodes for the FindNode result given a specified distance:

<https://github.com/erigontech/erigon/blob/v2.60.10/accounts/abi/type.go#L158>

```
func (t *UDPv5) collectTableNodes(rip net.IP, distances []uint, limit int) []*enode.Node {
	nodes := make([]*enode.Node, 0, len(distances))
	var processed = make(map[uint]struct{})
	for _, dist := range distances {
		// Reject duplicate / invalid distances.
		_, seen := processed[dist]
		if seen || dist > 256 {
			continue
		}


		// Get the nodes.
		var bn []*enode.Node
		if dist == 0 {
			bn = []*enode.Node{t.Self()}
		} else if dist <= 256 {
			t.tab.mutex.Lock()
			bn = unwrapNodes(t.tab.bucketAtDistance(int(dist)).entries)
			t.tab.mutex.Unlock()
		}
		processed[dist] = struct{}{}


		// Apply some pre-checks to avoid sending invalid nodes.
		for _, n := range bn {
			// TODO livenessChecks > 1
			if netutil.CheckRelayIP(rip, n.IP()) != nil {
				continue
			}
			nodes = append(nodes, n)
			if len(nodes) >= limit {
				return nodes
			}
		}
	}
	return nodes
}
```

However, it misses the liveness chen when collecting the nodes into the table, which is also mentioned as the TODO: (<https://github.com/erigontech/erigon/blob/v2.61.0-beta1/p2p/discover/v5\\_udp.go#L839> ）:

In this case, the node that has not been checked liveness will also be included in the table.

It is worth noted a similar issue has been fixed in go-ethereum: <https://github.com/ethereum/go-ethereum/pull/28686>

## Impact Details

.Nodes with no liveness check will be included in the node table.

## References

* <https://github.com/erigontech/erigon/tree/v2.61.0-beta1>
* <https://github.com/ethereum/go-ethereum/pull/28686>

## Proof of Concept

## Proof of Concept

Here we provide the following test case to show that nodes with no liveness check will be collected.

The default livenessChecks in function `wrapNode()` is zero, so there is no liveness check, which is used to mimic the nodes without liveness checks.

```
package discover


import (
   "bytes"
   "context"
   "crypto/ecdsa"
   "encoding/binary"
   "errors"
   "fmt"
   "net"
   "reflect"
   "runtime"
   "testing"
   "time"


   "github.com/ledgerwatch/erigon/turbo/testlog"
   "github.com/ledgerwatch/log/v3"


   "github.com/ledgerwatch/erigon/p2p/discover/v5wire"
   "github.com/ledgerwatch/erigon/p2p/enode"
   "github.com/ledgerwatch/erigon/p2p/enr"
   "github.com/ledgerwatch/erigon/rlp"
)



func wrapNode(n *enode.Node) *node {
   return &node{Node: *n}
}


func wrapNodes(ns []*enode.Node) []*node {
   result := make([]*node, len(ns))
   for i, n := range ns {
      result[i] = wrapNode(n)
   }
   return result
}

////////////unit test//////////

func TestCollectTableNodes(t *testing.T) {
   logger := log.New()
   test := newUDPV5Test(t, logger)
   t.Cleanup(test.close)

   nodes253 := nodesAtDistance(test.table.self().ID(), 253, 5)
   fillTable(test.table, wrapNodes(nodes253))

   rip := new(net.IP)
   distances := []uint{253}
   limit := 256

   nodes := test.udp.collectTableNodes(*rip, distances, limit)

   fmt.Printf("The collected nodes are: %v\n", nodes)
   fmt.Printf("Number of the collected nodes is: %d\n", len(nodes))

}
```

As the test result shows, all nodes without liveness check are also collected into the table:

```
=== RUN   TestCollectTableNodes
The collected nodes are: [enr:-DyAgIJpZIRudWxsgmlwhAEAAgGIbnVsbGFkZHKgN_Nh_UDMNfBEwoyLNAHxti9lG9dB0WZUoYuTrh3Fikg enr:-DyAgIJpZIRudWxsgmlwhAIAAgKIbnVsbGFkZHKgN8qNUV_UmPwblUu29QgouNp_lYSdQkXJ7WowYEHpX_0 enr:-DyAgIJpZIRudWxsgmlwhAMAAgOIbnVsbGFkZHKgN6JjlG7eFq-HMOlKgjkhSh0dzSAeuFeUfcTQSwF6qrQ enr:-DyAgIJpZIRudWxsgmlwhAQAAgSIbnVsbGFkZHKgN3xzWvF5XH-wIVucytScYrWLImpix0rdD5oFVO2Ytrc enr:-DyAgIJpZIRudWxsgmlwhAUAAgWIbnVsbGFkZHKgN5LIK4BmeON56kQ4ocADZHbe9pKkBfoOLctBpfgBpWM]
Number of the collected nodes is: 5
--- PASS: TestCollectTableNodes (0.04s)
PASS
```


---

# Agent Instructions: Querying This Documentation

If you need additional information that is not directly available in this page, you can query the documentation dynamically by asking a question.

Perform an HTTP GET request on the current page URL with the `ask` query parameter:

```
GET https://reports.immunefi.com/ethereum-protocol-or-attackathon/37352-bc-insight-missing-liveness-check-in-collecttablenodes.md?ask=<question>
```

The question should be specific, self-contained, and written in natural language.
The response will contain a direct answer to the question and relevant excerpts and sources from the documentation.

Use this mechanism when the answer is not explicitly present in the current page, you need clarification or additional context, or you want to retrieve related documentation sections.