CVE-2023-32675: Always-Incorrect Control Flow Implementation (CWE-670)
value: Vyper is a pythonic Smart Contract Language for the ethereum virtual machine. In contracts with more than one regular nonpayable function, it is possible to send funds to the default function, even if the default function is marked `nonpayable`. This applies to contracts compiled with vyper versions prior to 0.3.8. This issue was fixed by the removal of the global `calldatasize` check in commit `02339dfda`. Users are advised to upgrade to version 0.3.8. Users unable to upgrade should avoid use of nonpayable default functions.
CWE description: The code contains a control flow path that does not reflect the algorithm that the path is intended to implement, leading to incorrect behavior any time this path is navigated.
CVSS v3 base score: 5.3
Published: 2023-05-19T20:15Z
Repository: https://github.com/vyperlang/vyper
Language: Python
Files changed: tests/parser/features/test_init.py, vyper/codegen/function_definitions/external_function.py, vyper/codegen/module.py
Commit date: 2023-01-20 08:06:25 -0800
Commit message: refactor: optimize calldatasize check (#3104)

this changes the calldatasize check from a global/contract-entry check
to a function-local check - the calldatasize check only needs to be
present if a selector is 0. this maintains the invariant protecting
against the bug in #1603 while being more efficient.

Co-authored-by: Charles Cooper <cooper.charles.m@gmail.com>
Diff: diff --git a/tests/parser/features/test_init.py b/tests/parser/features/test_init.py
index 3ef7a440a5..feeabe311a 100644
--- a/tests/parser/features/test_init.py
+++ b/tests/parser/features/test_init.py
@@ -13,16 +13,17 @@ def __init__(a: uint256):
     c = get_contract(code, *[123])
 
     assert c.val() == 123
 
     # Make sure the init code does not access calldata
-    opcodes = vyper.compile_code(code, ["opcodes"])["opcodes"].split(" ")
-    ir_return_idx = opcodes.index("JUMP")
+    assembly = vyper.compile_code(code, ["asm"])["asm"].split(" ")
+    ir_return_idx_start = assembly.index("{")
+    ir_return_idx_end = assembly.index("}")
 
-    assert "CALLDATALOAD" in opcodes
-    assert "CALLDATACOPY" not in opcodes[:ir_return_idx]
-    assert "CALLDATALOAD" not in opcodes[:ir_return_idx]
+    assert "CALLDATALOAD" in assembly
+    assert "CALLDATACOPY" not in assembly[:ir_return_idx_start] + assembly[ir_return_idx_end:]
+    assert "CALLDATALOAD" not in assembly[:ir_return_idx_start] + assembly[ir_return_idx_end:]
 
 
 def test_init_calls_internal(get_contract, assert_compile_failed, assert_tx_failed):
     code = """
 foo: public(uint8)
diff --git a/vyper/codegen/function_definitions/external_function.py b/vyper/codegen/function_definitions/external_function.py
index 202c75a448..6de5bab16f 100644
--- a/vyper/codegen/function_definitions/external_function.py
+++ b/vyper/codegen/function_definitions/external_function.py
@@ -121,11 +121,28 @@ def _generate_kwarg_handlers(context: Context, sig: FunctionSignature) -> List[A
             copy_arg.source_pos = getpos(x.ast_source)
             ret.append(copy_arg)
 
         ret.append(["goto", sig.external_function_base_entry_label])
 
-        ret = ["if", ["eq", "_calldata_method_id", method_id], ret]
+        method_id_check = ["eq", "_calldata_method_id", method_id]
+
+        # if there is a function whose selector is 0, it won't be distinguished
+        # from the case where nil calldata is supplied, b/c calldataload loads
+        # 0s past the end of physical calldata (cf. yellow paper).
+        # since supplying 0 calldata is expected to trigger the fallback fn,
+        # we check that calldatasize > 0, which distinguishes the 0 selector
+        # from the fallback function "selector"
+        # (equiv. to "all selectors not in the selector table").
+
+        # note: cases where not enough calldata is supplied (besides
+        # calldatasize==0) are not addressed here b/c a calldatasize
+        # well-formedness check is already present in the function body
+        # as part of abi validation
+        if method_id.value == 0:
+            method_id_check = ["and", ["gt", "calldatasize", 0], method_id_check]
+
+        ret = ["if", method_id_check, ret]
         return ret
 
     ret = ["seq"]
 
     keyword_args = sig.default_args
diff --git a/vyper/codegen/module.py b/vyper/codegen/module.py
index fdca675819..71f9ed552d 100644
--- a/vyper/codegen/module.py
+++ b/vyper/codegen/module.py
@@ -118,13 +118,10 @@ def _runtime_ir(runtime_functions, all_sigs, global_ctx):
     # fallback label is the immediate next instruction,
     close_selector_section = ["goto", "fallback"]
 
     runtime = [
         "seq",
-        # check that calldatasize is at least 4, otherwise
-        # calldataload will load zeros (cf. yellow paper).
-        ["if", ["lt", "calldatasize", 4], ["goto", "fallback"]],
         ["with", "_calldata_method_id", shr(224, ["calldataload", 0]), selector_section],
         close_selector_section,
         ["label", "fallback", ["var_list"], fallback_ir],
     ]
 

Vulnerable code:     opcodes = vyper.compile_code(code, ["opcodes"])["opcodes"].split(" ")
    ir_return_idx = opcodes.index("JUMP")
    assert "CALLDATALOAD" in opcodes
    assert "CALLDATACOPY" not in opcodes[:ir_return_idx]
    assert "CALLDATALOAD" not in opcodes[:ir_return_idx]
        ret = ["if", ["eq", "_calldata_method_id", method_id], ret]
        # check that calldatasize is at least 4, otherwise
        # calldataload will load zeros (cf. yellow paper).
        ["if", ["lt", "calldatasize", 4], ["goto", "fallback"]],
Fixed code:     assembly = vyper.compile_code(code, ["asm"])["asm"].split(" ")
    ir_return_idx_start = assembly.index("{")
    ir_return_idx_end = assembly.index("}")
    assert "CALLDATALOAD" in assembly
    assert "CALLDATACOPY" not in assembly[:ir_return_idx_start] + assembly[ir_return_idx_end:]
    assert "CALLDATALOAD" not in assembly[:ir_return_idx_start] + assembly[ir_return_idx_end:]
        method_id_check = ["eq", "_calldata_method_id", method_id]

        # if there is a function whose selector is 0, it won't be distinguished
        # from the case where nil calldata is supplied, b/c calldataload loads
        # 0s past the end of physical calldata (cf. yellow paper).
        # since supplying 0 calldata is expected to trigger the fallback fn,
        # we check that calldatasize > 0, which distinguishes the 0 selector
        # from the fallback function "selector"
        # (equiv. to "all selectors not in the selector table").

        # note: cases where not enough calldata is supplied (besides
        # calldatasize==0) are not addressed here b/c a calldatasize
        # well-formedness check is already present in the function body
        # as part of abi validation
        if method_id.value == 0:
            method_id_check = ["and", ["gt", "calldatasize", 0], method_id_check]

        ret = ["if", method_id_check, ret]