fix[lang]: fix invalid memory read in raw_create (#4624)

`raw_create` could issue an invalid memory read, if the `value` kwarg
modifies the `initcode` argument (e.g., `DynArray.pop()`).

note that the invalid read was not observable in userspace, the program
would still behave correctly because the memory could not have been
written to in between the invalidation and the read

however, to be extra safe, this commit performs a memory copy if the
overlap is detected. this commit checks for overlap and issues a memory
copy if necessary. note that the overlap analysis also checks for
calls, which currently aren't problematic - the initcode is in memory,
which can't be affected by a reentrant call.

---------

Co-authored-by: Charles Cooper <[email protected]>

Author: cyberthirst

tests/functional/builtins/codegen/test_create_functions.py

@@ -785,6 +785,30 @@ def deploy_from_memory() -> address:
     assert env.get_code(res) == runtime
 
 
+# `initcode` and `value` arguments overlap
+def test_raw_create_memory_overlap(get_contract, env):
+    to_deploy_code = """
+foo: public(uint256)
+    """
+
+    out = compile_code(to_deploy_code, output_formats=["bytecode", "bytecode_runtime"])
+    initcode = bytes.fromhex(out["bytecode"].removeprefix("0x"))
+    runtime = bytes.fromhex(out["bytecode_runtime"].removeprefix("0x"))
+
+    deployer_code = """
+@external
+def deploy_from_calldata(s: Bytes[49152]) -> address:
+    v: DynArray[Bytes[49152], 2] = [s]
+    x: address = raw_create(v[0], value = 0 if v.pop() == b'' else 0, revert_on_failure=False)
+    return x
+    """
+
+    deployer = get_contract(deployer_code)
+
+    res = deployer.deploy_from_calldata(initcode)
+    assert env.get_code(res) == runtime
+
+
 def test_raw_create_double_eval(get_contract, env):
     to_deploy_code = """
 foo: public(uint256)
@@ -1095,6 +1119,8 @@ def __init__(arg: uint256):
     initcode = bytes.fromhex(out["bytecode"].removeprefix("0x"))
     runtime = bytes.fromhex(out["bytecode_runtime"].removeprefix("0x"))
 
+    # the implementation of `raw_create` firstly caches
+    # `value` and then `salt`, here the source order is `salt` then `value`
     deployer_code = """
 c: Bytes[1024]
 salt: bytes32
@@ -1122,3 +1148,33 @@ def deploy_from_calldata(s: Bytes[1024], arg: uint256, salt: bytes32, value_: ui
     assert HexBytes(res) == create2_address_of(deployer.address, salt, initcode)
     assert env.get_code(res) == runtime
     assert env.get_balance(res) == value
+
+
+# test vararg and kwarg order of evaluation
+# test fails because `value` gets evaluated
+# before the 1st vararg which doesn't follow
+# source code order
+@pytest.mark.xfail(raises=AssertionError)
+def test_raw_create_eval_order(get_contract):
+    code = """
+a: public(uint256)
+
+@deploy
+def __init__():
+    initcode: Bytes[100] = b"a"
+    res: address = raw_create(
+        initcode, self.test1(), value=self.test2(), revert_on_failure=False
+    )
+
+@internal
+def test1() -> uint256:
+    self.a = 1
+    return 1
+
+@internal
+def test2() -> uint256:
+    self.a = 2
+    return 2
+    """
+    c = get_contract(code)
+    assert c.a() == 2

vyper/builtins/functions.py

@@ -1602,6 +1602,14 @@ def _build_create_IR(self, expr, args, context, value, salt, revert_on_failure):
         initcode = args[0]
         ctor_args = args[1:]
 
+        if any(potential_overlap(initcode, other) for other in ctor_args + [value, salt]):
+            # value or salt could be expressions which trample the initcode
+            # buffer. cf. test_raw_create_memory_overlap
+            # note that potential_overlap is overly conservative, since it
+            # checks for the existence of calls (which are not applicable
+            # here, since `initcode` is guaranteed to be in memory).
+            initcode = create_memory_copy(initcode, context)
+
         # encode the varargs
         to_encode = ir_tuple_from_args(ctor_args)
         type_size_bound = to_encode.typ.abi_type.size_bound()