Common Weakness Enumeration

CWE-682

Discouraged

Incorrect Calculation

Abstraction: Pillar · Status: Draft

The product performs a calculation that generates incorrect or unintended results that are later used in security-critical decisions or resource management.

171 vulnerabilities reference this CWE, most recent first.

GHSA-XW37-57QP-9MM4

Vulnerability from github – Published: 2021-06-29 21:14 – Updated: 2023-02-09 19:40
VLAI
Summary
Consensus flaw during block processing in github.com/ethereum/go-ethereum
Details

Impact

A consensus-vulnerability in Geth could cause a chain split, where vulnerable versions refuse to accept the canonical chain.

Description

A flaw was repoted at 2020-08-11 by John Youngseok Yang (Software Platform Lab), where a particular sequence of transactions could cause a consensus failure.

  • Tx 1:
  • sender invokes caller.
  • caller invokes 0xaa. 0xaa has 3 wei, does a self-destruct-to-self
  • caller does a 1 wei -call to 0xaa, who thereby has 1 wei (the code in 0xaa still executed, since the tx is still ongoing, but doesn't redo the selfdestruct, it takes a different path if callvalue is non-zero)

  • Tx 2:

  • sender does a 5-wei call to 0xaa. No exec (since no code).

In geth, the result would be that 0xaa had 6 wei, whereas OE reported (correctly) 5 wei. Furthermore, in geth, if the second tx was not executed, the 0xaa would be destructed, resulting in 0 wei. Thus obviously wrong.

It was determined that the root cause was this commit from this PR. The semantics of createObject was subtly changd, into returning a non-nil object (with deleted=true) where it previously did not if the account had been destructed. This return value caused the new object to inherit the old balance:

func (s *StateDB) CreateAccount(addr common.Address) {
    newObj, prev := s.createObject(addr)
    if prev != nil {
        newObj.setBalance(prev.data.Balance)
    }
}

It was determined that the minimal possible correct fix was

+++ b/core/state/statedb.go
@@ -589,7 +589,10 @@ func (s *StateDB) createObject(addr common.Address) (newobj, prev *stateObject)
                s.journal.append(resetObjectChange{prev: prev, prevdestruct: prevdestruct})
        }
        s.setStateObject(newobj)
-       return newobj, prev
+       if prev != nil && !prev.deleted {
+               return newobj, prev
+       }
+       return newobj, nil

Patches

See above. The fix was included in Geth v1.9.20 "Paragade".

Credits

The bug was found by @johnyangk and reported via bounty@ethereum.org.

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

{
  "affected": [
    {
      "package": {
        "ecosystem": "Go",
        "name": "github.com/ethereum/go-ethereum"
      },
      "ranges": [
        {
          "events": [
            {
              "introduced": "1.9.4"
            },
            {
              "fixed": "1.9.20"
            }
          ],
          "type": "ECOSYSTEM"
        }
      ]
    }
  ],
  "aliases": [
    "CVE-2020-26265"
  ],
  "database_specific": {
    "cwe_ids": [
      "CWE-682"
    ],
    "github_reviewed": true,
    "github_reviewed_at": "2021-05-21T21:15:07Z",
    "nvd_published_at": null,
    "severity": "MODERATE"
  },
  "details": "### Impact\n\nA consensus-vulnerability in Geth could cause a chain split, where vulnerable versions refuse to accept the canonical chain. \n\n### Description\n\n\nA flaw was repoted at 2020-08-11 by John Youngseok Yang (Software Platform Lab), where a particular sequence of transactions could cause a consensus failure.\n\n- Tx 1:\n  - `sender` invokes `caller`.\n  - `caller` invokes `0xaa`. `0xaa` has 3 wei, does a self-destruct-to-self\n  - `caller` does a  `1 wei` -call to `0xaa`, who thereby has 1 wei (the code in `0xaa` still executed, since the tx is still ongoing, but doesn\u0027t redo the selfdestruct, it takes a different path if callvalue is non-zero)\n\n- Tx 2:\n  - `sender` does a 5-wei call to 0xaa. No exec (since no code). \n\nIn geth, the result would be that `0xaa` had `6 wei`, whereas OE reported (correctly) `5` wei. Furthermore, in geth, if the second tx was not executed, the `0xaa` would be destructed, resulting in `0 wei`. Thus obviously wrong. \n\nIt was determined that the root cause was this [commit](https://github.com/ethereum/go-ethereum/commit/223b950944f494a5b4e0957fd9f92c48b09037ad) from [this PR](https://github.com/ethereum/go-ethereum/pull/19953). The semantics of `createObject` was subtly changd, into returning a non-nil object (with `deleted=true`) where it previously did not if the account had been destructed. This return value caused the new object to inherit the old `balance`:\n\n```golang\nfunc (s *StateDB) CreateAccount(addr common.Address) {\n\tnewObj, prev := s.createObject(addr)\n\tif prev != nil {\n\t\tnewObj.setBalance(prev.data.Balance)\n\t}\n}\n```\n\nIt was determined that the minimal possible correct fix was\n\n```diff\n+++ b/core/state/statedb.go\n@@ -589,7 +589,10 @@ func (s *StateDB) createObject(addr common.Address) (newobj, prev *stateObject)\n                s.journal.append(resetObjectChange{prev: prev, prevdestruct: prevdestruct})\n        }\n        s.setStateObject(newobj)\n-       return newobj, prev\n+       if prev != nil \u0026\u0026 !prev.deleted {\n+               return newobj, prev\n+       }\n+       return newobj, nil\n```\n\n### Patches\n\nSee above. The fix was included in Geth `v1.9.20` \"Paragade\".\n\n### Credits\n\nThe bug was found by @johnyangk and reported via bounty@ethereum.org.\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)",
  "id": "GHSA-xw37-57qp-9mm4",
  "modified": "2023-02-09T19:40:33Z",
  "published": "2021-06-29T21:14:16Z",
  "references": [
    {
      "type": "WEB",
      "url": "https://github.com/ethereum/go-ethereum/security/advisories/GHSA-xw37-57qp-9mm4"
    },
    {
      "type": "ADVISORY",
      "url": "https://nvd.nist.gov/vuln/detail/CVE-2020-26265"
    },
    {
      "type": "WEB",
      "url": "https://github.com/ethereum/go-ethereum/pull/21080"
    },
    {
      "type": "WEB",
      "url": "https://github.com/ethereum/go-ethereum/pull/21409"
    },
    {
      "type": "WEB",
      "url": "https://github.com/ethereum/go-ethereum/commit/87c0ba92136a75db0ab2aba1046d4a9860375d6a"
    },
    {
      "type": "PACKAGE",
      "url": "https://github.com/ethereum/go-ethereum"
    },
    {
      "type": "WEB",
      "url": "https://github.com/ethereum/go-ethereum/releases/tag/v1.9.20"
    },
    {
      "type": "WEB",
      "url": "https://pkg.go.dev/vuln/GO-2021-0105"
    }
  ],
  "schema_version": "1.4.0",
  "severity": [
    {
      "score": "CVSS:3.1/AV:N/AC:H/PR:L/UI:N/S:U/C:N/I:H/A:N",
      "type": "CVSS_V3"
    }
  ],
  "summary": "Consensus flaw during block processing in github.com/ethereum/go-ethereum"
}

Mitigation
Implementation

Understand your programming language's underlying representation and how it interacts with numeric calculation. Pay close attention to byte size discrepancies, precision, signed/unsigned distinctions, truncation, conversion and casting between types, "not-a-number" calculations, and how your language handles numbers that are too large or too small for its underlying representation.

Mitigation MIT-8
Implementation

Strategy: Input Validation

Perform input validation on any numeric input by ensuring that it is within the expected range. Enforce that the input meets both the minimum and maximum requirements for the expected range.

Mitigation
Implementation

Use the appropriate type for the desired action. For example, in C/C++, only use unsigned types for values that could never be negative, such as height, width, or other numbers related to quantity.

Mitigation
Architecture and Design

Strategy: Language Selection

  • Use languages, libraries, or frameworks that make it easier to handle numbers without unexpected consequences.
  • Examples include safe integer handling packages such as SafeInt (C++) or IntegerLib (C or C++).
Mitigation
Architecture and Design

Strategy: Libraries or Frameworks

  • Use languages, libraries, or frameworks that make it easier to handle numbers without unexpected consequences.
  • Examples include safe integer handling packages such as SafeInt (C++) or IntegerLib (C or C++).
Mitigation MIT-26
Implementation

Strategy: Compilation or Build Hardening

Examine compiler warnings closely and eliminate problems with potential security implications, such as signed / unsigned mismatch in memory operations, or use of uninitialized variables. Even if the weakness is rarely exploitable, a single failure may lead to the compromise of the entire system.

CAPEC-128: Integer Attacks

An attacker takes advantage of the structure of integer variables to cause these variables to assume values that are not expected by an application. For example, adding one to the largest positive integer in a signed integer variable results in a negative number. Negative numbers may be illegal in an application and the application may prevent an attacker from providing them directly, but the application may not consider that adding two positive numbers can create a negative number do to the structure of integer storage formats.

CAPEC-129: Pointer Manipulation

This attack pattern involves an adversary manipulating a pointer within a target application resulting in the application accessing an unintended memory location. This can result in the crashing of the application or, for certain pointer values, access to data that would not normally be possible or the execution of arbitrary code. Since pointers are simply integer variables, Integer Attacks may often be used in Pointer Attacks.