Skip to content

Commit

Permalink
fix[codegen]: make_setter overlap in the presence of staticcall (#…
Browse files Browse the repository at this point in the history
…4128)

this commit fixes another overlap bug in `make_setter`. this is a
variant of the fixes in ad9c10b and 1c8349e, specifically
fixing an oversight in ad9c10b - when there is a `staticcall`
contained inside of `make_setter`, there can still be src/dst overlap,
due to read-only re-entrancy(!). this commit adds `staticcall` to the
list of "risky call" opcodes, and adds a poc test case (contributed by
@trocher).

---------

Co-authored-by: trocher <[email protected]>
  • Loading branch information
charles-cooper and trocher authored Jun 12, 2024
1 parent 21f7172 commit 7770967
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -2582,3 +2582,38 @@ def boo():
c = get_contract(code)

assert c.foo() == [1, 2, 3, 4]


def test_make_setter_staticcall(get_contract):
# variant of GH #3503
code = """
interface A:
def boo() -> uint256 : view
interface B:
def boo() -> uint256 : nonpayable
a: DynArray[uint256, 10]
@external
def foo() -> DynArray[uint256, 10]:
self.a = [3, 0, 0]
self.a = [1, 2, staticcall A(self).boo(), 4]
return self.a # bug returns [1, 2, 1, 4]
@external
def bar() -> DynArray[uint256, 10]:
self.a = [3, 0, 0]
self.a = [1, 2, extcall B(self).boo(), 4]
return self.a # returns [1, 2, 3, 4]
@external
@view
# @nonpayable
def boo() -> uint256:
return self.a[0]
"""
c = get_contract(code)

assert c.foo() == [1, 2, 3, 4]
assert c.bar() == [1, 2, 3, 4]
2 changes: 1 addition & 1 deletion vyper/codegen/ir_node.py
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,7 @@ def referenced_variables(self):

@cached_property
def contains_risky_call(self):
ret = self.value in ("call", "delegatecall", "create", "create2")
ret = self.value in ("call", "delegatecall", "staticcall", "create", "create2")

for arg in self.args:
ret |= arg.contains_risky_call
Expand Down

0 comments on commit 7770967

Please sign in to comment.