ghsa-59hh-656j-3p7v
Vulnerability from github
Published
2021-10-25 19:42
Modified
2023-08-29 18:55
Severity ?
5.7 (Medium) - CVSS:3.1/AV:N/AC:L/PR:L/UI:R/S:U/C:N/I:N/A:H
Summary
Geth Node Vulnerable to DoS via maliciously crafted p2p message
Details

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.

golang 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: golang // 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

```diff --- 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: * Open an issue in go-ethereum * Email us at security@ethereum.org

Show details on source website

To clipboard 

{
  "affected": [
    {
      "package": {
        "ecosystem": "Go",
        "name": "github.com/ethereum/go-ethereum"
      },
      "ranges": [
        {
          "events": [
            {
              "introduced": "0"
            },
            {
              "fixed": "1.10.9"
            }
          ],
          "type": "ECOSYSTEM"
        }
      ]
    }
  ],
  "aliases": [
    "CVE-2021-41173"
  ],
  "database_specific": {
    "cwe_ids": [
      "CWE-20"
    ],
    "github_reviewed": true,
    "github_reviewed_at": "2021-10-25T18:23:10Z",
    "nvd_published_at": "2021-10-26T14:15:00Z",
    "severity": "MODERATE"
  },
  "details": "### Impact\n\nA 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. \n\n### Details\n\nOn September 21, 2021, geth-team member Gary Rong (@rjl493456442) found a way to crash the snap request handler . \nBy using this vulnerability, a peer connected on the `snap/1` protocol could cause a vulnerable node to crash with a `panic`.\n\nIn the `trie.TryGetNode` implementation, if the requested path is reached, the associated node will be returned. However the nilness is\nnot checked there.\n\n```golang\nfunc (t *Trie) tryGetNode(origNode node, path []byte, pos int) (item []byte, newnode node, resolved int, err error) {\n\t// If we reached the requested path, return the current node\n\tif pos \u003e= len(path) {\n\t\t// Although we most probably have the original node expanded, encoding\n\t\t// that into consensus form can be nasty (needs to cascade down) and\n\t\t// time consuming. Instead, just pull the hash up from disk directly.\n\t\tvar hash hashNode\n\t\tif node, ok := origNode.(hashNode); ok {\n\t\t\thash = node\n\t\t} else {\n\t\t\thash, _ = origNode.cache()\n\t\t}\n```\nMore specifically the `origNode` can be nil(e.g. the child of fullnode) and system can panic at line `hash, _ = origNode.cache()`. \n\nWhen 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:\n```golang\n\t\t\t\t// Storage slots requested, open the storage trie and retrieve from there\n\t\t\t\taccount, err := snap.Account(common.BytesToHash(pathset[0]))\n\t\t\t\tloads++ // always account database reads, even for failures\n\t\t\t\tif account == nil {\n\t\t\t\t\tbreak\n\t\t\t\t}\n\t\t\t\tstTrie, err := trie.NewSecure(common.BytesToHash(account.Root), triedb)\n```\nThe 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`. \n\n### Patches\n\n```diff\n--- a/eth/protocols/snap/handler.go\n+++ b/eth/protocols/snap/handler.go\n@@ -469,7 +469,7 @@ func handleMessage(backend Backend, peer *Peer) error {\n \t\t\t\t// Storage slots requested, open the storage trie and retrieve from there\n \t\t\t\taccount, err := snap.Account(common.BytesToHash(pathset[0]))\n \t\t\t\tloads++ // always account database reads, even for failures\n-\t\t\t\tif err != nil {\n+\t\t\t\tif err != nil || account == nil {\n \t\t\t\t\tbreak\n \t\t\t\t}\n \t\t\t\tstTrie, err := trie.NewSecure(common.BytesToHash(account.Root), triedb)\ndiff --git a/trie/trie.go b/trie/trie.go\nindex 7ea7efa835..d0f0d4e2bc 100644\n--- a/trie/trie.go\n+++ b/trie/trie.go\n@@ -174,6 +174,10 @@ func (t *Trie) TryGetNode(path []byte) ([]byte, int, error) {\n }\n \n func (t *Trie) tryGetNode(origNode node, path []byte, pos int) (item []byte, newnode node, resolved int, err error) {\n+\t// If non-existent path requested, abort\n+\tif origNode == nil {\n+\t\treturn nil, nil, 0, nil\n+\t}\n \t// If we reached the requested path, return the current node\n \tif pos \u003e= len(path) {\n \t\t// Although we most probably have the original node expanded, encoding\n@@ -193,10 +197,6 @@ func (t *Trie) tryGetNode(origNode node, path []byte, pos int) (item []byte, new\n \t}\n \t// Path still needs to be traversed, descend into children\n \tswitch n := (origNode).(type) {\n-\tcase nil:\n-\t\t// Non-existent path requested, abort\n-\t\treturn nil, nil, 0, nil\n-\n \tcase valueNode:\n \t\t// Path prematurely ended, abort\n \t\treturn nil, nil, 0, nil\n\n``` \nThe fixes were merged into [#23657](https://github.com/ethereum/go-ethereum/pull/23657), with commit [f1fd963](https://github.com/ethereum/go-ethereum/pull/23657/commits/f1fd963a5a965e643e52fcf805a2a02a323c32b8), and released as part of Geth [v1.10.9](https://github.com/ethereum/go-ethereum/tree/v1.10.9) on Sept 29, 2021. \n\n### Workarounds\n\nApply the patch above or upgrade to a version which is not vulnerable.\n\n### For more information\nIf you have any questions or comments about this advisory:\n* Open an issue in [go-ethereum](https://github.com/ethereum/go-ethereum/)\n* Email us at [security@ethereum.org](mailto:security@ethereum.org)\n",
  "id": "GHSA-59hh-656j-3p7v",
  "modified": "2023-08-29T18:55:39Z",
  "published": "2021-10-25T19:42:57Z",
  "references": [
    {
      "type": "WEB",
      "url": "https://github.com/ethereum/go-ethereum/security/advisories/GHSA-59hh-656j-3p7v"
    },
    {
      "type": "ADVISORY",
      "url": "https://nvd.nist.gov/vuln/detail/CVE-2021-41173"
    },
    {
      "type": "WEB",
      "url": "https://github.com/ethereum/go-ethereum/pull/23657/commits/f1fd963a5a965e643e52fcf805a2a02a323c32b8"
    },
    {
      "type": "WEB",
      "url": "https://github.com/ethereum/go-ethereum/pull/23801"
    },
    {
      "type": "WEB",
      "url": "https://github.com/ethereum/go-ethereum/commit/e40b37718326b8b4873b3b00a0db2e6c6d9ea738"
    },
    {
      "type": "PACKAGE",
      "url": "https://github.com/ethereum/go-ethereum"
    },
    {
      "type": "WEB",
      "url": "https://github.com/ethereum/go-ethereum/releases/tag/v1.10.9"
    },
    {
      "type": "WEB",
      "url": "https://pkg.go.dev/vuln/GO-2022-0256"
    }
  ],
  "schema_version": "1.4.0",
  "severity": [
    {
      "score": "CVSS:3.1/AV:N/AC:L/PR:L/UI:R/S:U/C:N/I:N/A:H",
      "type": "CVSS_V3"
    }
  ],
  "summary": "Geth Node Vulnerable to DoS via maliciously crafted p2p message "
}


Log in or create an account to share your comment.




Tags
Taxonomy of the tags.




Sightings

Author Source Type Date

Nomenclature

  • Seen: The vulnerability was mentioned, discussed, or seen somewhere by the user.
  • Confirmed: The vulnerability is confirmed from an analyst perspective.
  • Exploited: This vulnerability was exploited and seen by the user reporting the sighting.
  • Patched: This vulnerability was successfully patched by the user reporting the sighting.
  • Not exploited: This vulnerability was not exploited or seen by the user reporting the sighting.
  • Not confirmed: The user expresses doubt about the veracity of the vulnerability.
  • Not patched: This vulnerability was not successfully patched by the user reporting the sighting.