peer: fix nil deref in newPingPayload on BestBlockHeader error
What the developer wrote
Authored by Elle Mouton
peer: fix nil deref in newPingPayload on BestBlockHeader error
The condition guarding the early return used && when it should have
used ||. When BestBlockHeader returns an error with a nil header, the
old code only short-circuited if the nil header equalled
lastBlockHeader. Otherwise it fell through to header.Serialize(),
causing a nil pointer dereference panic.
Change the condition to return the cached serialized header whenever
there is an error OR when the header is unchanged.
What changed, and why it matters
This commit fixes a simple but real programming bug: a logical 'and' was used where an 'or' was needed. In the LND lightning node software, when preparing a network ping message, the code asks for the latest Bitcoin block header. If that lookup fails and returns no header, the old code would try to use the missing header and crash the program with a nil pointer panic. The fix makes the code return a cached header whenever the lookup errors OR when the header hasn't changed, preventing the crash.
Apply the patch. It is a one-line correctness fix with low risk. Nodes should upgrade to avoid potential crashes during peer ping exchange when BestBlockHeader returns an error.
Security signals we found
nil pointer dereference panic in peer message handling
logic operator bug (&& vs ||) causing incorrect error handling
denial-of-service vector: unhandled error path in ping payload construction
Evidence from the diff
In peer/brontide.go, newPingPayload (invoked during ping message construction) calls BestBlockHeader() to obtain the current best block header. The original guard if err != nil && header == lastBlockHeader only returned early when both an error occurred and the returned header equaled the cached one. If BestBlockHeader returned (nil, err), the equality check would be false, so execution fell through to header.Serialize(), dereferencing a nil pointer and causing a panic. The patch changes the guard to if err != nil || header == lastBlockHeader, correctly returning the cached serialized header on any error or when the header is unchanged.
Changed components
peer/brontide.gonewPingPayload functionping message construction in peer connection logicInspect captured patch +1 / −1
diff --git a/peer/brontide.go b/peer/brontide.go
index d624ac9..336b88e 100644
--- a/peer/brontide.go
+++ b/peer/brontide.go
@@ -734,7 +734,7 @@ func NewBrontide(cfg Config) *Brontide {
// used to cross-check our own view of the network to mitigate
// various types of eclipse attacks.
header, err := p.cfg.BestBlockView.BestBlockHeader()
- if err != nil && header == lastBlockHeader {
+ if err != nil || header == lastBlockHeader {
return lastSerializedBlockHeader[:]
}
Why this scored 42/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.