CVE-2023-37902: Unchecked Return Value (CWE-252)
value: Vyper is a Pythonic programming language that targets the Ethereum Virtual Machine (EVM). Prior to version 0.3.10, the ecrecover precompile does not fill the output buffer if the signature does not verify. However, the ecrecover builtin will still return whatever is at memory location 0. This means that the if the compiler has been convinced to write to the 0 memory location with specially crafted data (generally, this can happen with a hashmap access or immutable read) just before the ecrecover, a signature check might pass on an invalid signature. Version 0.3.10 contains a patch for this issue.
CWE description: The product does not check the return value from a method or function, which can prevent it from detecting unexpected states and conditions.
CVSS v3 base score: 5.3
Published: 2023-07-25T21:15Z
Repository: https://github.com/vyperlang/vyper
Language: Python
Files changed: tests/parser/functions/test_ecrecover.py, vyper/builtins/functions.py
Commit date: 2023-07-25 07:41:12 -0700
Commit message: Merge pull request from GHSA-f5x6-7qgp-jhf3


Diff: diff --git a/tests/parser/functions/test_ecrecover.py b/tests/parser/functions/test_ecrecover.py
index 77e9655b3e..40c9a6a936 100644
--- a/tests/parser/functions/test_ecrecover.py
+++ b/tests/parser/functions/test_ecrecover.py
@@ -38,5 +38,23 @@ def test_ecrecover_uints2() -> address:
     assert c.test_ecrecover_uints(h, sig.v, sig.r, sig.s) == local_account.address
     assert c.test_ecrecover2() == local_account.address
     assert c.test_ecrecover_uints2() == local_account.address
 
     print("Passed ecrecover test")
+
+
+def test_invalid_signature(get_contract):
+    code = """
+dummies: HashMap[address, HashMap[address, uint256]]
+
+@external
+def test_ecrecover(hash: bytes32, v: uint8, r: uint256) -> address:
+    # read from hashmap to put garbage in 0 memory location
+    s: uint256 = self.dummies[msg.sender][msg.sender]
+    return ecrecover(hash, v, r, s)
+    """
+    c = get_contract(code)
+    hash_ = bytes(i for i in range(32))
+    v = 0  # invalid v! ecrecover precompile will not write to output buffer
+    r = 0
+    # note web3.py decoding of 0x000..00 address is None.
+    assert c.test_ecrecover(hash_, v, r) is None
diff --git a/vyper/builtins/functions.py b/vyper/builtins/functions.py
index e1dcee6b8d..685d832c01 100644
--- a/vyper/builtins/functions.py
+++ b/vyper/builtins/functions.py
@@ -762,33 +762,23 @@ class ECRecover(BuiltinFunction):
         v_t, r_t, s_t = [get_possible_types_from_node(arg).pop() for arg in node.args[1:]]
         return [BYTES32_T, v_t, r_t, s_t]
 
     @process_inputs
     def build_IR(self, expr, args, kwargs, context):
-        placeholder_node = IRnode.from_list(
-            context.new_internal_variable(BytesT(128)), typ=BytesT(128), location=MEMORY
-        )
+        input_buf = context.new_internal_variable(get_type_for_exact_size(128))
+        output_buf = MemoryPositions.FREE_VAR_SPACE
         return IRnode.from_list(
             [
                 "seq",
-                ["mstore", placeholder_node, args[0]],
-                ["mstore", ["add", placeholder_node, 32], args[1]],
-                ["mstore", ["add", placeholder_node, 64], args[2]],
-                ["mstore", ["add", placeholder_node, 96], args[3]],
-                [
-                    "pop",
-                    [
-                        "staticcall",
-                        ["gas"],
-                        1,
-                        placeholder_node,
-                        128,
-                        MemoryPositions.FREE_VAR_SPACE,
-                        32,
-                    ],
-                ],
-                ["mload", MemoryPositions.FREE_VAR_SPACE],
+                # clear output memory first, ecrecover can return 0 bytes
+                ["mstore", output_buf, 0],
+                ["mstore", input_buf, args[0]],
+                ["mstore", input_buf + 32, args[1]],
+                ["mstore", input_buf + 64, args[2]],
+                ["mstore", input_buf + 96, args[3]],
+                ["staticcall", "gas", 1, input_buf, 128, output_buf, 32],
+                ["mload", output_buf],
             ],
             typ=AddressT(),
         )
 
 

Vulnerable code:         placeholder_node = IRnode.from_list(
            context.new_internal_variable(BytesT(128)), typ=BytesT(128), location=MEMORY
        )
                ["mstore", placeholder_node, args[0]],
                ["mstore", ["add", placeholder_node, 32], args[1]],
                ["mstore", ["add", placeholder_node, 64], args[2]],
                ["mstore", ["add", placeholder_node, 96], args[3]],
                [
                    "pop",
                    [
                        "staticcall",
                        ["gas"],
                        1,
                        placeholder_node,
                        128,
                        MemoryPositions.FREE_VAR_SPACE,
                        32,
                    ],
                ],
                ["mload", MemoryPositions.FREE_VAR_SPACE],
Fixed code: 

def test_invalid_signature(get_contract):
    code = """
dummies: HashMap[address, HashMap[address, uint256]]

@external
def test_ecrecover(hash: bytes32, v: uint8, r: uint256) -> address:
    # read from hashmap to put garbage in 0 memory location
    s: uint256 = self.dummies[msg.sender][msg.sender]
    return ecrecover(hash, v, r, s)
    """
    c = get_contract(code)
    hash_ = bytes(i for i in range(32))
    v = 0  # invalid v! ecrecover precompile will not write to output buffer
    r = 0
    # note web3.py decoding of 0x000..00 address is None.
    assert c.test_ecrecover(hash_, v, r) is None
        input_buf = context.new_internal_variable(get_type_for_exact_size(128))
        output_buf = MemoryPositions.FREE_VAR_SPACE
                # clear output memory first, ecrecover can return 0 bytes
                ["mstore", output_buf, 0],
                ["mstore", input_buf, args[0]],
                ["mstore", input_buf + 32, args[1]],
                ["mstore", input_buf + 64, args[2]],
                ["mstore", input_buf + 96, args[3]],
                ["staticcall", "gas", 1, input_buf, 128, output_buf, 32],
                ["mload", output_buf],