Vyper vulnerable to memory corruption in certain builtins utilizing `msize`

@@ -143,7 +143,9 @@
     (["sub", "x", 0], ["x"]),
     (["sub", "x", "x"], [0]),
     (["sub", ["sload", 0], ["sload", 0]], None),
-    (["sub", ["callvalue"], ["callvalue"]], None),
+    (["sub", ["callvalue"], ["callvalue"]], [0]),
+    (["sub", ["msize"], ["msize"]], None),
+    (["sub", ["gas"], ["gas"]], None),
     (["sub", -1, ["sload", 0]], ["not", ["sload", 0]]),
     (["mul", "x", 1], ["x"]),
     (["div", "x", 1], ["x"]),
@@ -210,7 +212,9 @@
     (["eq", -1, ["add", -(2**255), 2**255 - 1]], [1]),  # test compile-time wrapping
     (["eq", -2, ["add", 2**256 - 1, 2**256 - 1]], [1]),  # test compile-time wrapping
     (["eq", "x", "x"], [1]),
-    (["eq", "callvalue", "callvalue"], None),
+    (["eq", "gas", "gas"], None),
+    (["eq", "msize", "msize"], None),
+    (["eq", "callvalue", "callvalue"], [1]),
     (["ne", "x", "x"], [0]),
 ]
 

@@ -431,3 +431,212 @@ def test2(target: address, salt: bytes32) -> address:
     # test2 = c.test2(b"\x01", salt)
     # assert HexBytes(test2) == create2_address_of(c.address, salt, vyper_initcode(b"\x01"))
     # assert_tx_failed(lambda: c.test2(bytecode, salt))
+
+
+# XXX: these various tests to check the msize allocator for
+# create_copy_of and create_from_blueprint depend on calling convention
+# and variables writing to memory. think of ways to make more robust to
+# changes in calling convention and memory layout
+@pytest.mark.parametrize("blueprint_prefix", [b"", b"\xfe", b"\xfe\71\x00"])
+def test_create_from_blueprint_complex_value(
+    get_contract, deploy_blueprint_for, w3, blueprint_prefix
+):
+    # check msize allocator does not get trampled by value= kwarg
+    code = """
+var: uint256
+
+@external
+@payable
+def __init__(x: uint256):
+    self.var = x
+
+@external
+def foo()-> uint256:
+    return self.var
+    """
+
+    prefix_len = len(blueprint_prefix)
+
+    some_constant = b"\00" * 31 + b"\x0c"
+
+    deployer_code = f"""
+created_address: public(address)
+x: constant(Bytes[32]) = {some_constant}
+
+@internal
+def foo() -> uint256:
+    g:uint256 = 42
+    return 3
+
+@external
+@payable
+def test(target: address):
+    self.created_address = create_from_blueprint(
+        target,
+        x,
+        code_offset={prefix_len},
+        value=self.foo(),
+        raw_args=True
+    )
+    """
+
+    foo_contract = get_contract(code, 12)
+    expected_runtime_code = w3.eth.get_code(foo_contract.address)
+
+    f, FooContract = deploy_blueprint_for(code, initcode_prefix=blueprint_prefix)
+
+    d = get_contract(deployer_code)
+
+    d.test(f.address, transact={"value": 3})
+
+    test = FooContract(d.created_address())
+    assert w3.eth.get_code(test.address) == expected_runtime_code
+    assert test.foo() == 12
+
+
+@pytest.mark.parametrize("blueprint_prefix", [b"", b"\xfe", b"\xfe\71\x00"])
+def test_create_from_blueprint_complex_salt_raw_args(
+    get_contract, deploy_blueprint_for, w3, blueprint_prefix
+):
+    # test msize allocator does not get trampled by salt= kwarg
+    code = """
+var: uint256
+
+@external
+@payable
+def __init__(x: uint256):
+    self.var = x
+
+@external
+def foo()-> uint256:
+    return self.var
+    """
+
+    some_constant = b"\00" * 31 + b"\x0c"
+    prefix_len = len(blueprint_prefix)
+
+    deployer_code = f"""
+created_address: public(address)
+
+x: constant(Bytes[32]) = {some_constant}
+salt: constant(bytes32) = keccak256("kebab")
+
+@internal
+def foo() -> bytes32:
+    g:uint256 = 42
+    return salt
+
+@external
+@payable
+def test(target: address):
+    self.created_address = create_from_blueprint(
+        target,
+        x,
+        code_offset={prefix_len},
+        salt=self.foo(),
+        raw_args= True
+    )
+    """
+
+    foo_contract = get_contract(code, 12)
+    expected_runtime_code = w3.eth.get_code(foo_contract.address)
+
+    f, FooContract = deploy_blueprint_for(code, initcode_prefix=blueprint_prefix)
+
+    d = get_contract(deployer_code)
+
+    d.test(f.address, transact={})
+
+    test = FooContract(d.created_address())
+    assert w3.eth.get_code(test.address) == expected_runtime_code
+    assert test.foo() == 12
+
+
+@pytest.mark.parametrize("blueprint_prefix", [b"", b"\xfe", b"\xfe\71\x00"])
+def test_create_from_blueprint_complex_salt_no_constructor_args(
+    get_contract, deploy_blueprint_for, w3, blueprint_prefix
+):
+    # test msize allocator does not get trampled by salt= kwarg
+    code = """
+var: uint256
+
+@external
+@payable
+def __init__():
+    self.var = 12
+
+@external
+def foo()-> uint256:
+    return self.var
+    """
+
+    prefix_len = len(blueprint_prefix)
+    deployer_code = f"""
+created_address: public(address)
+
+salt: constant(bytes32) = keccak256("kebab")
+
+@external
+@payable
+def test(target: address):
+    self.created_address = create_from_blueprint(
+        target,
+        code_offset={prefix_len},
+        salt=keccak256(_abi_encode(target))
+    )
+    """
+
+    foo_contract = get_contract(code)
+    expected_runtime_code = w3.eth.get_code(foo_contract.address)
+
+    f, FooContract = deploy_blueprint_for(code, initcode_prefix=blueprint_prefix)
+
+    d = get_contract(deployer_code)
+
+    d.test(f.address, transact={})
+
+    test = FooContract(d.created_address())
+    assert w3.eth.get_code(test.address) == expected_runtime_code
+    assert test.foo() == 12
+
+
+def test_create_copy_of_complex_kwargs(get_contract, w3):
+    # test msize allocator does not get trampled by salt= kwarg
+    complex_salt = """
+created_address: public(address)
+
+@external
+def test(target: address) -> address:
+    self.created_address = create_copy_of(
+        target,
+        salt=keccak256(_abi_encode(target))
+    )
+    return self.created_address
+
+    """
+
+    c = get_contract(complex_salt)
+    bytecode = w3.eth.get_code(c.address)
+    c.test(c.address, transact={})
+    test1 = c.created_address()
+    assert w3.eth.get_code(test1) == bytecode
+
+    # test msize allocator does not get trampled by value= kwarg
+    complex_value = """
+created_address: public(address)
+
+@external
+@payable
+def test(target: address) -> address:
+    value: uint256 = 2
+    self.created_address = create_copy_of(target, value = [2,2,2][value])
+    return self.created_address
+
+    """
+
+    c = get_contract(complex_value)
+    bytecode = w3.eth.get_code(c.address)
+
+    c.test(c.address, transact={"value": 2})
+    test1 = c.created_address()
+    assert w3.eth.get_code(test1) == bytecode

@@ -426,6 +426,164 @@ def baz(_addr: address, should_raise: bool) -> uint256:
     assert caller.baz(target.address, False) == 3
 
 
+# XXX: these test_raw_call_clean_mem* tests depend on variables and
+# calling convention writing to memory. think of ways to make more
+# robust to changes to calling convention and memory layout.
+
+
+def test_raw_call_msg_data_clean_mem(get_contract):
+    # test msize uses clean memory and does not get overwritten by
+    # any raw_call() arguments
+    code = """
+identity: constant(address) = 0x0000000000000000000000000000000000000004
+
+@external
+def foo():
+    pass
+
+@internal
+@view
+def get_address()->address:
+    a:uint256 = 121 # 0x79
+    return identity
+@external
+def bar(f: uint256, u: uint256) -> Bytes[100]:
+    # embed an internal call in the calculation of address
+    a: Bytes[100] = raw_call(self.get_address(), msg.data, max_outsize=100)
+    return a
+    """
+
+    c = get_contract(code)
+    assert (
+        c.bar(1, 2).hex() == "ae42e951"
+        "0000000000000000000000000000000000000000000000000000000000000001"
+        "0000000000000000000000000000000000000000000000000000000000000002"
+    )
+
+
+def test_raw_call_clean_mem2(get_contract):
+    # test msize uses clean memory and does not get overwritten by
+    # any raw_call() arguments, another way
+    code = """
+buf: Bytes[100]
+
+@external
+def bar(f: uint256, g: uint256, h: uint256) -> Bytes[100]:
+    # embed a memory modifying expression in the calculation of address
+    self.buf = raw_call(
+        [0x0000000000000000000000000000000000000004,][f-1],
+        msg.data,
+        max_outsize=100
+    )
+    return self.buf
+    """
+    c = get_contract(code)
+
+    assert (
+        c.bar(1, 2, 3).hex() == "9309b76e"
+        "0000000000000000000000000000000000000000000000000000000000000001"
+        "0000000000000000000000000000000000000000000000000000000000000002"
+        "0000000000000000000000000000000000000000000000000000000000000003"
+    )
+
+
+def test_raw_call_clean_mem3(get_contract):
+    # test msize uses clean memory and does not get overwritten by
+    # any raw_call() arguments, and also test order of evaluation for
+    # scope_multi
+    code = """
+buf: Bytes[100]
+canary: String[32]
+
+@internal
+def bar() -> address:
+    self.canary = "bar"
+    return 0x0000000000000000000000000000000000000004
+
+@internal
+def goo() -> uint256:
+    self.canary = "goo"
+    return 0
+
+@external
+def foo() -> String[32]:
+    self.buf = raw_call(self.bar(), msg.data, value = self.goo(), max_outsize=100)
+    return self.canary
+    """
+    c = get_contract(code)
+    assert c.foo() == "goo"
+
+
+def test_raw_call_clean_mem_kwargs_value(get_contract):
+    # test msize uses clean memory and does not get overwritten by
+    # any raw_call() kwargs
+    code = """
+buf: Bytes[100]
+
+# add a dummy function to trigger memory expansion in the selector table routine
+@external
+def foo():
+    pass
+
+@internal
+def _value() -> uint256:
+    x: uint256 = 1
+    return x
+
+@external
+def bar(f: uint256) -> Bytes[100]:
+    # embed a memory modifying expression in the calculation of address
+    self.buf = raw_call(
+        0x0000000000000000000000000000000000000004,
+        msg.data,
+        max_outsize=100,
+        value=self._value()
+    )
+    return self.buf
+    """
+    c = get_contract(code, value=1)
+
+    assert (
+        c.bar(13).hex() == "0423a132"
+        "000000000000000000000000000000000000000000000000000000000000000d"
+    )
+
+
+def test_raw_call_clean_mem_kwargs_gas(get_contract):
+    # test msize uses clean memory and does not get overwritten by
+    # any raw_call() kwargs
+    code = """
+buf: Bytes[100]
+
+# add a dummy function to trigger memory expansion in the selector table routine
+@external
+def foo():
+    pass
+
+@internal
+def _gas() -> uint256:
+    x: uint256 = msg.gas
+    return x
+
+@external
+def bar(f: uint256) -> Bytes[100]:
+    # embed a memory modifying expression in the calculation of address
+    self.buf = raw_call(
+        0x0000000000000000000000000000000000000004,
+        msg.data,
+        max_outsize=100,
+        gas=self._gas()
+    )
+    return self.buf
+    """
+    c = get_contract(code, value=1)
+
+    assert (
+        c.bar(15).hex() == "0423a132"
+        "000000000000000000000000000000000000000000000000000000000000000f"
+    )
+
+
 uncompilable_code = [
     (
         """

@@ -21,6 +21,7 @@
     clamp_basetype,
     clamp_nonzero,
     copy_bytes,
+    dummy_node_for_type,
     ensure_in_memory,
     eval_once_check,
     eval_seq,
@@ -36,7 +37,7 @@
     unwrap_location,
 )
 from vyper.codegen.expr import Expr
-from vyper.codegen.ir_node import Encoding
+from vyper.codegen.ir_node import Encoding, scope_multi
 from vyper.codegen.keccak256_helper import keccak256_helper
 from vyper.evm.address_space import MEMORY, STORAGE
 from vyper.exceptions import (
@@ -1155,14 +1156,17 @@ def build_IR(self, expr, args, kwargs, context):
             outsize,
         ]
 
-        if delegate_call:
-            call_op = ["delegatecall", gas, to, *common_call_args]
-        elif static_call:
-            call_op = ["staticcall", gas, to, *common_call_args]
-        else:
-            call_op = ["call", gas, to, value, *common_call_args]
+        gas, value = IRnode.from_list(gas), IRnode.from_list(value)
+        with scope_multi((to, value, gas), ("_to", "_value", "_gas")) as (b1, (to, value, gas)):
+            if delegate_call:
+                call_op = ["delegatecall", gas, to, *common_call_args]
+            elif static_call:
+                call_op = ["staticcall", gas, to, *common_call_args]
+            else:
+                call_op = ["call", gas, to, value, *common_call_args]
 
-        call_ir += [call_op]
+            call_ir += [call_op]
+            call_ir = b1.resolve(call_ir)
 
         # build sequence IR
         if outsize:
@@ -1589,13 +1593,15 @@ def build_IR(self, expr, context):
 
 # CREATE* functions
 
+CREATE2_SENTINEL = dummy_node_for_type(BYTES32_T)
+
 
 # create helper functions
 # generates CREATE op sequence + zero check for result
-def _create_ir(value, buf, length, salt=None, checked=True):
+def _create_ir(value, buf, length, salt, checked=True):
     args = [value, buf, length]
     create_op = "create"
-    if salt is not None:
+    if salt is not CREATE2_SENTINEL:
         create_op = "create2"
         args.append(salt)
 
@@ -1713,8 +1719,9 @@ def build_IR(self, expr, args, kwargs, context):
         context.check_is_not_constant("use {self._id}", expr)
 
         should_use_create2 = "salt" in [kwarg.arg for kwarg in expr.keywords]
+
         if not should_use_create2:
-            kwargs["salt"] = None
+            kwargs["salt"] = CREATE2_SENTINEL
 
         ir_builder = self._build_create_IR(expr, args, context, **kwargs)
 
@@ -1794,13 +1801,16 @@ def _add_gas_estimate(self, args, should_use_create2):
     def _build_create_IR(self, expr, args, context, value, salt):
         target = args[0]
 
-        with target.cache_when_complex("create_target") as (b1, target):
+        # something we can pass to scope_multi
+        with scope_multi(
+            (target, value, salt), ("create_target", "create_value", "create_salt")
+        ) as (b1, (target, value, salt)):
             codesize = IRnode.from_list(["extcodesize", target])
             msize = IRnode.from_list(["msize"])
-            with codesize.cache_when_complex("target_codesize") as (
+            with scope_multi((codesize, msize), ("target_codesize", "mem_ofst")) as (
                 b2,
-                codesize,
-            ), msize.cache_when_complex("mem_ofst") as (b3, mem_ofst):
+                (codesize, mem_ofst),
+            ):
                 ir = ["seq"]
 
                 # make sure there is actually code at the target
@@ -1824,7 +1834,7 @@ def _build_create_IR(self, expr, args, context, value, salt):
 
                 ir.append(_create_ir(value, buf, buf_len, salt))
 
-                return b1.resolve(b2.resolve(b3.resolve(ir)))
+                return b1.resolve(b2.resolve(ir))
 
 
 class CreateFromBlueprint(_CreateBase):
@@ -1877,17 +1887,18 @@ def _build_create_IR(self, expr, args, context, value, salt, code_offset, raw_ar
         # (since the abi encoder could write to fresh memory).
         # it would be good to not require the memory copy, but need
         # to evaluate memory safety.
-        with target.cache_when_complex("create_target") as (b1, target), argslen.cache_when_complex(
-            "encoded_args_len"
-        ) as (b2, encoded_args_len), code_offset.cache_when_complex("code_ofst") as (b3, codeofst):
-            codesize = IRnode.from_list(["sub", ["extcodesize", target], codeofst])
+        with scope_multi(
+            (target, value, salt, argslen, code_offset),
+            ("create_target", "create_value", "create_salt", "encoded_args_len", "code_offset"),
+        ) as (b1, (target, value, salt, encoded_args_len, code_offset)):
+            codesize = IRnode.from_list(["sub", ["extcodesize", target], code_offset])
             # copy code to memory starting from msize. we are clobbering
             # unused memory so it's safe.
             msize = IRnode.from_list(["msize"], location=MEMORY)
-            with codesize.cache_when_complex("target_codesize") as (
-                b4,
-                codesize,
-            ), msize.cache_when_complex("mem_ofst") as (b5, mem_ofst):
+            with scope_multi((codesize, msize), ("target_codesize", "mem_ofst")) as (
+                b2,
+                (codesize, mem_ofst),
+            ):
                 ir = ["seq"]
 
                 # make sure there is code at the target, and that
@@ -1907,7 +1918,7 @@ def _build_create_IR(self, expr, args, context, value, salt, code_offset, raw_ar
                 # copy the target code into memory.
                 # layout starting from mem_ofst:
                 # 00...00 (22 0's) | preamble | bytecode
-                ir.append(["extcodecopy", target, mem_ofst, codeofst, codesize])
+                ir.append(["extcodecopy", target, mem_ofst, code_offset, codesize])
 
                 ir.append(copy_bytes(add_ofst(mem_ofst, codesize), argbuf, encoded_args_len, bufsz))
 
@@ -1922,7 +1933,7 @@ def _build_create_IR(self, expr, args, context, value, salt, code_offset, raw_ar
 
                 ir.append(_create_ir(value, mem_ofst, length, salt))
 
-                return b1.resolve(b2.resolve(b3.resolve(b4.resolve(b5.resolve(ir)))))
+                return b1.resolve(b2.resolve(ir))
 
 
 class _UnsafeMath(BuiltinFunction):

@@ -1,3 +1,4 @@
+import contextlib
 import re
 from enum import Enum, auto
 from functools import cached_property
@@ -46,6 +47,77 @@ class Encoding(Enum):
     # future: packed
 
 
+# shortcut for chaining multiple cache_when_complex calls
+# CMC 2023-08-10 remove this and scope_together _as soon as_ we have
+# real variables in IR (that we can declare without explicit scoping -
+# needs liveness analysis).
+@contextlib.contextmanager
+def scope_multi(ir_nodes, names):
+    assert len(ir_nodes) == len(names)
+
+    builders = []
+    scoped_ir_nodes = []
+
+    class _MultiBuilder:
+        def resolve(self, body):
+            # sanity check that it's initialized properly
+            assert len(builders) == len(ir_nodes)
+            ret = body
+            for b in reversed(builders):
+                ret = b.resolve(ret)
+            return ret
+
+    mb = _MultiBuilder()
+
+    with contextlib.ExitStack() as stack:
+        for arg, name in zip(ir_nodes, names):
+            b, ir_node = stack.enter_context(arg.cache_when_complex(name))
+
+            builders.append(b)
+            scoped_ir_nodes.append(ir_node)
+
+        yield mb, scoped_ir_nodes
+
+
+# create multiple with scopes if any of the items are complex, to force
+# ordering of side effects.
+@contextlib.contextmanager
+def scope_together(ir_nodes, names):
+    assert len(ir_nodes) == len(names)
+
+    should_scope = any(s._optimized.is_complex_ir for s in ir_nodes)
+
+    class _Builder:
+        def resolve(self, body):
+            if not should_scope:
+                # uses of the variable have already been inlined
+                return body
+
+            ret = body
+            # build with scopes from inside-out (hence reversed)
+            for arg, name in reversed(list(zip(ir_nodes, names))):
+                ret = ["with", name, arg, ret]
+
+            if isinstance(body, IRnode):
+                return IRnode.from_list(
+                    ret, typ=body.typ, location=body.location, encoding=body.encoding
+                )
+            else:
+                return ret
+
+    b = _Builder()
+
+    if should_scope:
+        ir_vars = tuple(
+            IRnode.from_list(name, typ=arg.typ, location=arg.location, encoding=arg.encoding)
+            for (arg, name) in zip(ir_nodes, names)
+        )
+        yield b, ir_vars
+    else:
+        # inline them
+        yield b, ir_nodes
+
+
 # this creates a magical block which maps to IR `with`
 class _WithBuilder:
     def __init__(self, ir_node, name, should_inline=False):
@@ -326,14 +398,15 @@ def _check(condition, err):
     def gas(self):
         return self._gas + self.add_gas_estimate
 
-    # the IR should be cached.
-    # TODO make this private. turns out usages are all for the caching
-    # idiom that cache_when_complex addresses
+    # the IR should be cached and/or evaluated exactly once
     @property
     def is_complex_ir(self):
         # list of items not to cache. note can add other env variables
         # which do not change, e.g. calldatasize, coinbase, etc.
-        do_not_cache = {"~empty", "calldatasize"}
+        # reads (from memory or storage) should not be cached because
+        # they can have or be affected by side effects.
+        do_not_cache = {"~empty", "calldatasize", "callvalue"}
+
         return (
             isinstance(self.value, str)
             and (self.value.lower() in VALID_IR_MACROS or self.value.upper() in get_ir_opcodes())