CVE-2022-29255: Always-Incorrect Control Flow Implementation (CWE-670)
value: Vyper is a Pythonic Smart Contract Language for the ethereum virtual machine. In versions prior to 0.3.4 when a calling an external contract with no return value, the contract address (including side effects) could be evaluated twice. This may result in incorrect outcomes for contracts. This issue has been addressed in v0.3.4.
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: 7.5
Published: 2022-06-09T09:15Z
Repository: https://github.com/vyperlang/vyper
Language: Python
Files changed: tests/parser/features/external_contracts/test_external_contract_calls.py, vyper/codegen/external_call.py
Commit date: 2022-06-06 11:38:28 -0700
Commit message: Merge pull request from GHSA-4v9q-cgpw-cf38

in external call codegen, when `extcodesize` is called on the target
address, the IR for evaluating the target address can be evaluated
twice. this can result in any side effects (embedded in the evaluation
of the target address) being executed twice.
Diff: diff --git a/tests/parser/features/external_contracts/test_external_contract_calls.py b/tests/parser/features/external_contracts/test_external_contract_calls.py
index 513c359f0..b9408cf6c 100644
--- a/tests/parser/features/external_contracts/test_external_contract_calls.py
+++ b/tests/parser/features/external_contracts/test_external_contract_calls.py
@@ -2409,5 +2409,46 @@ def bar(foo: Foo):
     c = get_contract(code)
 
     # fails due to returndatasize being nonzero but also lt 64
     assert_tx_failed(lambda: c.bar(bad_1.address))
     c.bar(bad_2.address)
+
+
+def test_contract_address_evaluation(get_contract):
+    callee_code = """
+# implements: Foo
+
+interface Counter:
+    def increment_counter(): nonpayable
+
+@external
+def foo():
+    pass
+
+@external
+def bar() -> address:
+    Counter(msg.sender).increment_counter()
+    return self
+    """
+    code = """
+# implements: Counter
+
+interface Foo:
+    def foo(): nonpayable
+    def bar() -> address: nonpayable
+
+counter: uint256
+
+@external
+def increment_counter():
+    self.counter += 1
+
+@external
+def do_stuff(f: Foo) -> uint256:
+    Foo(f.bar()).foo()
+    return self.counter
+    """
+
+    c1 = get_contract(code)
+    c2 = get_contract(callee_code)
+
+    assert c1.do_stuff(c2.address) == 1
diff --git a/vyper/codegen/external_call.py b/vyper/codegen/external_call.py
index c72ee10be..e177a69b9 100644
--- a/vyper/codegen/external_call.py
+++ b/vyper/codegen/external_call.py
@@ -166,19 +166,11 @@ def _parse_kwargs(call_expr, context):
 
 def _extcodesize_check(address):
     return ["assert", ["extcodesize", address]]
 
 
-def ir_for_external_call(call_expr, context):
-    from vyper.codegen.expr import Expr  # TODO rethink this circular import
-
-    contract_address = Expr.parse_value_expr(call_expr.func.value, context)
-    call_kwargs = _parse_kwargs(call_expr, context)
-    args_ir = [Expr(x, context).ir_node for x in call_expr.args]
-
-    assert isinstance(contract_address.typ, InterfaceType)
-
+def _external_call_helper(contract_address, args_ir, call_kwargs, call_expr, context):
     # expr.func._metadata["type"].return_type is more accurate
     # than fn_sig.return_type in the case of JSON interfaces.
     fn_type = call_expr.func._metadata["type"]
 
     # sanity check
@@ -221,5 +213,19 @@ def ir_for_external_call(call_expr, context):
     if fn_type.return_type is not None:
         return_t = new_type_to_old_type(fn_type.return_type)
         ret.append(ret_unpacker)
 
     return IRnode.from_list(ret, typ=return_t, location=MEMORY)
+
+
+def ir_for_external_call(call_expr, context):
+    from vyper.codegen.expr import Expr  # TODO rethink this circular import
+
+    contract_address = Expr.parse_value_expr(call_expr.func.value, context)
+    assert isinstance(contract_address.typ, InterfaceType)
+    args_ir = [Expr(x, context).ir_node for x in call_expr.args]
+    call_kwargs = _parse_kwargs(call_expr, context)
+
+    with contract_address.cache_when_complex("external_contract") as (b1, contract_address):
+        return b1.resolve(
+            _external_call_helper(contract_address, args_ir, call_kwargs, call_expr, context)
+        )

Vulnerable code: def ir_for_external_call(call_expr, context):
    from vyper.codegen.expr import Expr  # TODO rethink this circular import

    contract_address = Expr.parse_value_expr(call_expr.func.value, context)
    call_kwargs = _parse_kwargs(call_expr, context)
    args_ir = [Expr(x, context).ir_node for x in call_expr.args]

    assert isinstance(contract_address.typ, InterfaceType)

Fixed code: 

def test_contract_address_evaluation(get_contract):
    callee_code = """
# implements: Foo

interface Counter:
    def increment_counter(): nonpayable

@external
def foo():
    pass

@external
def bar() -> address:
    Counter(msg.sender).increment_counter()
    return self
    """
    code = """
# implements: Counter

interface Foo:
    def foo(): nonpayable
    def bar() -> address: nonpayable

counter: uint256

@external
def increment_counter():
    self.counter += 1

@external
def do_stuff(f: Foo) -> uint256:
    Foo(f.bar()).foo()
    return self.counter
    """

    c1 = get_contract(code)
    c2 = get_contract(callee_code)

    assert c1.do_stuff(c2.address) == 1
def _external_call_helper(contract_address, args_ir, call_kwargs, call_expr, context):


def ir_for_external_call(call_expr, context):
    from vyper.codegen.expr import Expr  # TODO rethink this circular import

    contract_address = Expr.parse_value_expr(call_expr.func.value, context)
    assert isinstance(contract_address.typ, InterfaceType)
    args_ir = [Expr(x, context).ir_node for x in call_expr.args]
    call_kwargs = _parse_kwargs(call_expr, context)

    with contract_address.cache_when_complex("external_contract") as (b1, contract_address):
        return b1.resolve(
            _external_call_helper(contract_address, args_ir, call_kwargs, call_expr, context)
        )