fix[codegen]: panic on potential eval order issue for some builtins (#…

…4157)

`extract32()` and `slice()` have an evaluation order issue when
the arguments touch the same data. specifically, the length and data
evaluation are interleaved with the index/start/length evaluations. in
unusual situations (such as those in the included test cases), this
can result in "invalid" reads where the data and length reads appear
out of order. this commit conservatively blocks compilation if the
preconditions for the interleaved evaluation are detected.

---------

Co-authored-by: trocher <[email protected]>
Co-authored-by: cyberthirst <[email protected]>

tests/functional/builtins/codegen/test_extract32.py

@@ -1,6 +1,7 @@
 import pytest
 
 from vyper.evm.opcodes import version_check
+from vyper.exceptions import CompilerPanic
 
 
 @pytest.mark.parametrize("location", ["storage", "transient"])
@@ -98,3 +99,50 @@ def foq(inp: Bytes[32]) -> address:
 
     with tx_failed():
         c.foq(b"crow" * 8)
+
+
+# to fix in future release
+@pytest.mark.xfail(raises=CompilerPanic, reason="risky overlap")
+def test_extract32_order_of_eval(get_contract):
+    extract32_code = """
+var:DynArray[Bytes[96], 1]
+
+@internal
+def bar() -> uint256:
+    self.var[0] = b'hellohellohellohellohellohellohello'
+    self.var.pop()
+    return 3
+
+@external
+def foo() -> bytes32:
+    self.var = [b'abcdefghijklmnopqrstuvwxyz123456789']
+    return extract32(self.var[0], self.bar(), output_type=bytes32)
+    """
+
+    c = get_contract(extract32_code)
+    assert c.foo() == b"defghijklmnopqrstuvwxyz123456789"
+
+
+# to fix in future release
+@pytest.mark.xfail(raises=CompilerPanic, reason="risky overlap")
+def test_extract32_order_of_eval_extcall(get_contract):
+    slice_code = """
+var:DynArray[Bytes[96], 1]
+
+interface Bar:
+    def bar() -> uint256: payable
+
+@external
+def bar() -> uint256:
+    self.var[0] = b'hellohellohellohellohellohellohello'
+    self.var.pop()
+    return 3
+
+@external
+def foo() -> bytes32:
+    self.var = [b'abcdefghijklmnopqrstuvwxyz123456789']
+    return extract32(self.var[0], extcall Bar(self).bar(), output_type=bytes32)
+    """
+
+    c = get_contract(slice_code)
+    assert c.foo() == b"defghijklmnopqrstuvwxyz123456789"

tests/functional/builtins/codegen/test_slice.py

@@ -5,7 +5,7 @@
 from vyper.compiler import compile_code
 from vyper.compiler.settings import OptimizationLevel, Settings
 from vyper.evm.opcodes import version_check
-from vyper.exceptions import ArgumentException, TypeMismatch
+from vyper.exceptions import ArgumentException, CompilerPanic, TypeMismatch
 
 _fun_bytes32_bounds = [(0, 32), (3, 29), (27, 5), (0, 5), (5, 3), (30, 2)]
 
@@ -562,3 +562,53 @@ def foo(cs: String[64]) -> uint256:
     c = get_contract(code)
     # ensure that counter was incremented only once
     assert c.foo(arg) == 1
+
+
+# to fix in future release
+@pytest.mark.xfail(raises=CompilerPanic, reason="risky overlap")
+def test_slice_order_of_eval(get_contract):
+    slice_code = """
+var:DynArray[Bytes[96], 1]
+
+interface Bar:
+    def bar() -> uint256: payable
+
+@external
+def bar() -> uint256:
+    self.var[0] = b'hellohellohellohellohellohellohello'
+    self.var.pop()
+    return 32
+
+@external
+def foo() -> Bytes[96]:
+    self.var = [b'abcdefghijklmnopqrstuvwxyz123456789']
+    return slice(self.var[0], 3, extcall Bar(self).bar())
+    """
+
+    c = get_contract(slice_code)
+    assert c.foo() == b"defghijklmnopqrstuvwxyz123456789"
+
+
+# to fix in future release
+@pytest.mark.xfail(raises=CompilerPanic, reason="risky overlap")
+def test_slice_order_of_eval2(get_contract):
+    slice_code = """
+var:DynArray[Bytes[96], 1]
+
+interface Bar:
+    def bar() -> uint256: payable
+
+@external
+def bar() -> uint256:
+    self.var[0] = b'hellohellohellohellohellohellohello'
+    self.var.pop()
+    return 3
+
+@external
+def foo() -> Bytes[96]:
+    self.var = [b'abcdefghijklmnopqrstuvwxyz123456789']
+    return slice(self.var[0], extcall Bar(self).bar(), 32)
+    """
+
+    c = get_contract(slice_code)
+    assert c.foo() == b"defghijklmnopqrstuvwxyz123456789"

vyper/builtins/functions.py

@@ -29,6 +29,7 @@
     get_type_for_exact_size,
     ir_tuple_from_args,
     make_setter,
+    potential_overlap,
     promote_signed_int,
     sar,
     shl,
@@ -357,6 +358,9 @@ def build_IR(self, expr, args, kwargs, context):
             assert is_bytes32, src
             src = ensure_in_memory(src, context)
 
+        if potential_overlap(src, start) or potential_overlap(src, length):
+            raise CompilerPanic("risky overlap")
+
         with src.cache_when_complex("src") as (b1, src), start.cache_when_complex("start") as (
             b2,
             start,
@@ -862,6 +866,9 @@ def build_IR(self, expr, args, kwargs, context):
         bytez, index = args
         ret_type = kwargs["output_type"]
 
+        if potential_overlap(bytez, index):
+            raise CompilerPanic("risky overlap")
+
         def finalize(ret):
             annotation = "extract32"
             ret = IRnode.from_list(ret, typ=ret_type, annotation=annotation)