From ba5b836722c8931048b86a12b72df49696cf83a5 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Wed, 26 Feb 2025 10:52:07 +0100 Subject: [PATCH 01/10] fix[codegen]: relax the filter for augassign oob check --- vyper/codegen/ir_node.py | 12 ++++++++++++ vyper/codegen/stmt.py | 4 +++- vyper/semantics/analysis/base.py | 1 + 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/vyper/codegen/ir_node.py b/vyper/codegen/ir_node.py index 6ed6345674..38cd41a84d 100644 --- a/vyper/codegen/ir_node.py +++ b/vyper/codegen/ir_node.py @@ -498,6 +498,18 @@ def contains_risky_call(self): return ret + @cached_property + def contains_writeable_call(self): + ret = self.value in ("call", "delegatecall", "create", "create2") + + for arg in self.args: + ret |= arg.contains_writeable_call + + if getattr(self, "is_self_call", False): + ret |= self.invoked_function_ir.func_ir.contains_writeable_call + + return ret + @cached_property def contains_self_call(self): return getattr(self, "is_self_call", False) or any(x.contains_self_call for x in self.args) diff --git a/vyper/codegen/stmt.py b/vyper/codegen/stmt.py index 388dfba629..ffda836373 100644 --- a/vyper/codegen/stmt.py +++ b/vyper/codegen/stmt.py @@ -293,7 +293,9 @@ def parse_AugAssign(self): if var.typ._is_prim_word: continue # oob - GHSA-4w26-8p97-f4jp - if var in right.variable_writes or right.contains_risky_call: + if var in right.variable_writes or ( + var.is_state_variable() and right.contains_writeable_call + ): raise CodegenPanic("unreachable") with target.cache_when_complex("_loc") as (b, target): diff --git a/vyper/semantics/analysis/base.py b/vyper/semantics/analysis/base.py index adfc7540a0..c7bfbc11aa 100644 --- a/vyper/semantics/analysis/base.py +++ b/vyper/semantics/analysis/base.py @@ -205,6 +205,7 @@ def set_position(self, position: VarOffset) -> None: assert isinstance(position, VarOffset) # sanity check self.position = position + # TODO: convert to property def is_state_variable(self): non_state_locations = (DataLocation.UNSET, DataLocation.MEMORY, DataLocation.CALLDATA) # `self` gets a VarInfo, but it is not considered a state From 6393b2f1f008f96da58b9dc12b7dfd4610a750a0 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Wed, 26 Feb 2025 11:07:12 +0100 Subject: [PATCH 02/10] update test --- .../codegen/features/test_assignment.py | 47 +++++++------------ 1 file changed, 17 insertions(+), 30 deletions(-) diff --git a/tests/functional/codegen/features/test_assignment.py b/tests/functional/codegen/features/test_assignment.py index 53d02dfbff..242fd00231 100644 --- a/tests/functional/codegen/features/test_assignment.py +++ b/tests/functional/codegen/features/test_assignment.py @@ -99,36 +99,6 @@ def test_augassign_oob(get_contract, tx_failed, source): c.poc() -@pytest.mark.parametrize( - "source", - [ - """ -a: public(DynArray[uint256, 2]) - -interface Foo: - def foo() -> uint256: view - -@external -def foo() -> uint256: - return self.a[1] - -@external -def entry() -> DynArray[uint256, 2]: - self.a = [1, 1] - # panics due to staticcall - self.a[1] += staticcall Foo(self).foo() - return self.a - """ - ], -) -@pytest.mark.xfail(strict=True, raises=CodegenPanic) -def test_augassign_rhs_references_lhs(get_contract, tx_failed, source): - # xfail here (with panic): - c = get_contract(source) - - assert c.entry() == [1, 2] - - @pytest.mark.parametrize( "source", [ @@ -160,6 +130,23 @@ def entry() -> DynArray[uint256, 2]: self.a = [1, 1] self.a[1] += self.read() return self.a + """, + """ +a: public(DynArray[uint256, 2]) + +interface Foo: + def foo() -> uint256: view + +@external +def foo() -> uint256: + return self.a[1] + +@external +def entry() -> DynArray[uint256, 2]: + self.a = [1, 1] + # panics due to staticcall + self.a[1] += staticcall Foo(self).foo() + return self.a """, ], ) From 57385ae75b820e75af833696b988e42a2cbdc4c6 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Wed, 26 Feb 2025 11:19:34 +0100 Subject: [PATCH 03/10] add a test --- .../codegen/features/test_assignment.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/functional/codegen/features/test_assignment.py b/tests/functional/codegen/features/test_assignment.py index 242fd00231..d49d00c705 100644 --- a/tests/functional/codegen/features/test_assignment.py +++ b/tests/functional/codegen/features/test_assignment.py @@ -132,6 +132,25 @@ def entry() -> DynArray[uint256, 2]: return self.a """, """ +interface Foo: + def foo() -> uint256: nonpayable + +def get_foo() -> uint256: + return extcall Foo(self).foo() + +@external +def foo() -> uint256: + return 1 + +@external +def entry() -> DynArray[uint256, 2]: + # memory variable, can't be overwritten by extcall + a: DynArray[uint256, 2] = [1, 1] + # extcall hidden inside internal function + a[1] += self.get_foo() + return a + """, + """ a: public(DynArray[uint256, 2]) interface Foo: From 4151f0c62270617ca94e4a31c1f931913d97d9b1 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Wed, 26 Feb 2025 11:20:01 +0100 Subject: [PATCH 04/10] remove a comment --- tests/functional/codegen/features/test_assignment.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/functional/codegen/features/test_assignment.py b/tests/functional/codegen/features/test_assignment.py index d49d00c705..7014f8c7e9 100644 --- a/tests/functional/codegen/features/test_assignment.py +++ b/tests/functional/codegen/features/test_assignment.py @@ -163,7 +163,6 @@ def foo() -> uint256: @external def entry() -> DynArray[uint256, 2]: self.a = [1, 1] - # panics due to staticcall self.a[1] += staticcall Foo(self).foo() return self.a """, From 44da06b4572871f71d903ff56b1d5c20c19f56eb Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Wed, 26 Feb 2025 11:24:06 +0100 Subject: [PATCH 05/10] add another test --- .../codegen/features/test_assignment.py | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/tests/functional/codegen/features/test_assignment.py b/tests/functional/codegen/features/test_assignment.py index 7014f8c7e9..d82d08cb05 100644 --- a/tests/functional/codegen/features/test_assignment.py +++ b/tests/functional/codegen/features/test_assignment.py @@ -135,6 +135,22 @@ def entry() -> DynArray[uint256, 2]: interface Foo: def foo() -> uint256: nonpayable +@external +def foo() -> uint256: + return 1 + +@external +def entry() -> DynArray[uint256, 2]: + # memory variable, can't be overwritten by extcall, so there + # is no panic + a: DynArray[uint256, 2] = [1, 1] + a[1] += extcall Foo(self).foo() + return a + """, + """ +interface Foo: + def foo() -> uint256: nonpayable + def get_foo() -> uint256: return extcall Foo(self).foo() @@ -144,7 +160,8 @@ def foo() -> uint256: @external def entry() -> DynArray[uint256, 2]: - # memory variable, can't be overwritten by extcall + # memory variable, can't be overwritten by extcall, so there + # is no panic a: DynArray[uint256, 2] = [1, 1] # extcall hidden inside internal function a[1] += self.get_foo() From dbdd54da9508b333545b06b976202bb441dc127f Mon Sep 17 00:00:00 2001 From: cyberthirst Date: Wed, 26 Feb 2025 12:06:56 +0100 Subject: [PATCH 06/10] add transient overlap tests --- .../codegen/features/test_assignment.py | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/tests/functional/codegen/features/test_assignment.py b/tests/functional/codegen/features/test_assignment.py index d82d08cb05..83daec7aeb 100644 --- a/tests/functional/codegen/features/test_assignment.py +++ b/tests/functional/codegen/features/test_assignment.py @@ -190,6 +190,63 @@ def test_augassign_rhs_references_lhs2(get_contract, source): assert c.entry() == [1, 2] +@pytest.mark.requires_evm_version("cancun") +def test_augassign_rhs_references_lhs_transient(get_contract): + source = """ +x: transient(DynArray[uint256, 2]) + +def read() -> uint256: + return self.x[0] + +@external +def entry() -> DynArray[uint256, 2]: + self.x = [1, 1] + self.x[1] += self.read() + self.x[0] += self.x[1] + return self.x + """ + c = get_contract(source) + + assert c.entry() == [3, 2] + + +@pytest.mark.parametrize( + "source", + [ + """ +x: transient(DynArray[uint256, 2]) + +def write() -> uint256: + return self.x.pop() + +@external +def entry() -> DynArray[uint256, 2]: + self.x = [1, 1] + self.x[1] += self.write() + return self.x + """, + """ +x: transient(DynArray[uint256, 2]) + +@external +def entry() -> DynArray[uint256, 2]: + self.x = [1, 1] + self.x[1] += self.x.pop() + return self.x + """, + ], +) +@pytest.mark.requires_evm_version("cancun") +@pytest.mark.xfail(strict=True, raises=CodegenPanic) +def test_augassign_rhs_references_lhs_transient2(get_contract, tx_failed, source): + # xfail here (with panic): + c = get_contract(source) + + # not reached until the panic is fixed + with tx_failed(c): + c.entry() + + @pytest.mark.parametrize( "typ,in_val,out_val", [ From c2fdbd9b6c6999de1ed8606f887f69e7b54a02bf Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Wed, 26 Feb 2025 12:25:01 +0100 Subject: [PATCH 07/10] clarify tests --- tests/functional/codegen/features/test_assignment.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/functional/codegen/features/test_assignment.py b/tests/functional/codegen/features/test_assignment.py index 83daec7aeb..fb7bad0203 100644 --- a/tests/functional/codegen/features/test_assignment.py +++ b/tests/functional/codegen/features/test_assignment.py @@ -201,8 +201,10 @@ def read() -> uint256: @external def entry() -> DynArray[uint256, 2]: self.x = [1, 1] - self.x[1] += self.read() - self.x[0] += self.x[1] + # test augassign with state read hidden behind function call + self.x[0] += self.read() + # augassign with direct state read + self.x[1] += self.x[0] return self.x """ c = get_contract(source) @@ -222,6 +224,7 @@ def write() -> uint256: @external def entry() -> DynArray[uint256, 2]: self.x = [1, 1] + # hide state write behind function call self.x[1] += self.write() return self.x """, @@ -231,6 +234,7 @@ def entry() -> DynArray[uint256, 2]: @external def entry() -> DynArray[uint256, 2]: self.x = [1, 1] + # direct state write self.x[1] += self.x.pop() return self.x """, From 6a5c3037d4704dd3f036af1a6fff405fd5e81c91 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Wed, 26 Feb 2025 16:54:10 +0100 Subject: [PATCH 08/10] fix a test --- tests/functional/codegen/features/test_assignment.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functional/codegen/features/test_assignment.py b/tests/functional/codegen/features/test_assignment.py index fb7bad0203..c14fad5497 100644 --- a/tests/functional/codegen/features/test_assignment.py +++ b/tests/functional/codegen/features/test_assignment.py @@ -209,7 +209,7 @@ def entry() -> DynArray[uint256, 2]: """ c = get_contract(source) - assert c.entry() == [3, 2] + assert c.entry() == [2, 3] @pytest.mark.parametrize( From 42382ba9409c7e8b779c23e63e092b3cd3cce80c Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Thu, 27 Feb 2025 10:56:28 +0100 Subject: [PATCH 09/10] fix test for pre-cancun --- tests/functional/codegen/features/test_assignment.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/functional/codegen/features/test_assignment.py b/tests/functional/codegen/features/test_assignment.py index c14fad5497..cb0e33d72c 100644 --- a/tests/functional/codegen/features/test_assignment.py +++ b/tests/functional/codegen/features/test_assignment.py @@ -1,5 +1,6 @@ import pytest +from vyper.evm.opcodes import version_check from vyper.exceptions import CodegenPanic, ImmutableViolation, InvalidType, TypeMismatch @@ -240,9 +241,11 @@ def entry() -> DynArray[uint256, 2]: """, ], ) -@pytest.mark.requires_evm_version("cancun") @pytest.mark.xfail(strict=True, raises=CodegenPanic) def test_augassign_rhs_references_lhs_transient2(get_contract, tx_failed, source): + if not version_check(begin="cancun"): + # no transient available before cancun + pytest.skip() # xfail here (with panic): c = get_contract(source) From be05638763e8e9019c44329346d1ae3da29f863e Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Thu, 27 Feb 2025 12:01:36 +0100 Subject: [PATCH 10/10] whitespace --- tests/functional/codegen/features/test_assignment.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/functional/codegen/features/test_assignment.py b/tests/functional/codegen/features/test_assignment.py index cb0e33d72c..d74db6f5a7 100644 --- a/tests/functional/codegen/features/test_assignment.py +++ b/tests/functional/codegen/features/test_assignment.py @@ -246,6 +246,7 @@ def test_augassign_rhs_references_lhs_transient2(get_contract, tx_failed, source if not version_check(begin="cancun"): # no transient available before cancun pytest.skip() + # xfail here (with panic): c = get_contract(source)