Impact
A vulnerable node is susceptible to crash when processing a maliciously crafted message from a peer, via the snap/1 protocol. The crash can be triggered by sending a malicious snap/1 GetTrieNodes package.
Details
On September 21, 2021, geth-team member Gary Rong (@rjl493456442) found a way to crash the snap request handler .
By using this vulnerability, a peer connected on the snap/1 protocol could cause a vulnerable node to crash with a panic.
In the trie.TryGetNode implementation, if the requested path is reached, the associated node will be returned. However the nilness is
not checked there.
func (t *Trie) tryGetNode(origNode node, path []byte, pos int) (item []byte, newnode node, resolved int, err error) {
// If we reached the requested path, return the current node
if pos >= len(path) {
// Although we most probably have the original node expanded, encoding
// that into consensus form can be nasty (needs to cascade down) and
// time consuming. Instead, just pull the hash up from disk directly.
var hash hashNode
if node, ok := origNode.(hashNode); ok {
hash = node
} else {
hash, _ = origNode.cache()
}More specifically the origNode can be nil(e.g. the child of fullnode) and system can panic at line hash, _ = origNode.cache().
When investigating this, @holiman tried to find it via fuzzing, which uncovered a second crasher, also related to the snap GetTrieNodes package. If the caller requests a storage trie:
// Storage slots requested, open the storage trie and retrieve from there
account, err := snap.Account(common.BytesToHash(pathset[0]))
loads++ // always account database reads, even for failures
if account == nil {
break
}
stTrie, err := trie.NewSecure(common.BytesToHash(account.Root), triedb)The code assumes that snap.Account returns either a non-nil response unless error is also provided. This is however not the case, since snap.Account can return nil, nil.
Patches
--- a/eth/protocols/snap/handler.go
+++ b/eth/protocols/snap/handler.go
@@ -469,7 +469,7 @@ func handleMessage(backend Backend, peer *Peer) error {
// Storage slots requested, open the storage trie and retrieve from there
account, err := snap.Account(common.BytesToHash(pathset[0]))
loads++ // always account database reads, even for failures
- if err != nil {
+ if err != nil || account == nil {
break
}
stTrie, err := trie.NewSecure(common.BytesToHash(account.Root), triedb)
diff --git a/trie/trie.go b/trie/trie.go
index 7ea7efa835..d0f0d4e2bc 100644
--- a/trie/trie.go
+++ b/trie/trie.go
@@ -174,6 +174,10 @@ func (t *Trie) TryGetNode(path []byte) ([]byte, int, error) {
}
func (t *Trie) tryGetNode(origNode node, path []byte, pos int) (item []byte, newnode node, resolved int, err error) {
+ // If non-existent path requested, abort
+ if origNode == nil {
+ return nil, nil, 0, nil
+ }
// If we reached the requested path, return the current node
if pos >= len(path) {
// Although we most probably have the original node expanded, encoding
@@ -193,10 +197,6 @@ func (t *Trie) tryGetNode(origNode node, path []byte, pos int) (item []byte, new
}
// Path still needs to be traversed, descend into children
switch n := (origNode).(type) {
- case nil:
- // Non-existent path requested, abort
- return nil, nil, 0, nil
-
case valueNode:
// Path prematurely ended, abort
return nil, nil, 0, nil
The fixes were merged into #23657, with commit f1fd963, and released as part of Geth v1.10.9 on Sept 29, 2021.
Workarounds
Apply the patch above or upgrade to a version which is not vulnerable.
For more information
If you have any questions or comments about this advisory:
References
rjl493456442 Analyst
holiman Analyst
Impact
A vulnerable node is susceptible to crash when processing a maliciously crafted message from a peer, via the
snap/1protocol. The crash can be triggered by sending a malicioussnap/1GetTrieNodespackage.Details
On September 21, 2021, geth-team member Gary Rong (@rjl493456442) found a way to crash the snap request handler .
By using this vulnerability, a peer connected on the
snap/1protocol could cause a vulnerable node to crash with apanic.In the
trie.TryGetNodeimplementation, if the requested path is reached, the associated node will be returned. However the nilness isnot checked there.
More specifically the
origNodecan be nil(e.g. the child of fullnode) and system can panic at linehash, _ = origNode.cache().When investigating this, @holiman tried to find it via fuzzing, which uncovered a second crasher, also related to the snap
GetTrieNodespackage. If the caller requests a storage trie:The code assumes that
snap.Accountreturns either a non-nil response unlesserroris also provided. This is however not the case, sincesnap.Accountcan returnnil, nil.Patches
The fixes were merged into #23657, with commit f1fd963, and released as part of Geth v1.10.9 on Sept 29, 2021.
Workarounds
Apply the patch above or upgrade to a version which is not vulnerable.
For more information
If you have any questions or comments about this advisory:
References