From b32c1d6bccec957e8a81f83c687083d55bc2c1bf Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Thu, 20 Jun 2024 06:39:17 -0400 Subject: [PATCH 1/5] fix[codegen]: fix false positive in contains_risky_call the `potential_overlap` and `read_write_overlap()` functions use `contains_risky_call` to detect if there is potential for reentrancy. however, when the target is a precompile, there is no chance for reentrancy, so we filter them out of the detector. --- vyper/codegen/ir_node.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/vyper/codegen/ir_node.py b/vyper/codegen/ir_node.py index 6f9eb0359b..3981b83837 100644 --- a/vyper/codegen/ir_node.py +++ b/vyper/codegen/ir_node.py @@ -13,6 +13,8 @@ from vyper.semantics.types import VyperType from vyper.utils import VALID_IR_MACROS, ceil32 +PRECOMPILE_RANGE = (1, 10) + # Set default string representation for ints in IR output. AS_HEX_DEFAULT = False @@ -479,9 +481,21 @@ def variable_writes(self): return ret + @property + def is_call(self): + return self.value in ("call", "staticcall", "delegatecall") + + @property + def is_precompile_call(self): + assert self.is_call + target = self.args[1].value + lo, hi = PRECOMPILE_RANGE + return isinstance(target, int) and (lo <= target <= hi) + @cached_property def contains_risky_call(self): - ret = self.value in ("call", "delegatecall", "staticcall", "create", "create2") + ret = self.value in ("create", "create2") + ret |= self.is_call and not self.is_precompile_call for arg in self.args: ret |= arg.contains_risky_call From d950f1bd5f0b857832890b28aa880ea76bacb96b Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Thu, 20 Jun 2024 06:48:51 -0400 Subject: [PATCH 2/5] add tests --------- Co-authored-by: Hubert Ritzdorf Co-authored-by: trocher --- .../codegen/types/test_dynamic_array.py | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/tests/functional/codegen/types/test_dynamic_array.py b/tests/functional/codegen/types/test_dynamic_array.py index 2a0f4e77e5..17764e5237 100644 --- a/tests/functional/codegen/types/test_dynamic_array.py +++ b/tests/functional/codegen/types/test_dynamic_array.py @@ -1903,3 +1903,54 @@ def foo(): c = get_contract(code) with tx_failed(): c.foo() + + +# should not trip risky overlap detector +def test_risky_call_precompile(): + code = """ +x: DynArray[uint256, 32] + +@deploy +def __init__(): + self.x = [1, 1, 1] + +@internal +def bar() -> uint256: + self.x[0] = 1 + return 0 + +@external +def foo() -> uint256: + self.x[ + abi_decode( + slice( + concat( + abi_encode( + empty(uint256), + method_id=method_id("foo()")), + empty(bytes32) + ), 0, 32), + (uint256) + )] = self.bar() + return 0 + """ + assert compile_code(code) is not None + + +def test_risky_call_precompile2(): + code = """ +x: DynArray[uint256, 32] + +@internal +def bar() -> uint256: + a: DynArray[uint256, 100] = [] + b: DynArray[uint256, 100] = [] + a = b + return 0 + +@external +def foo() -> uint256: + c: uint256 = self.x[self.bar()] + return 0 + """ + assert compile_code(code) is not None From 3c622952f1762be48dd8dae0450c3ffd7eef705f Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Thu, 20 Jun 2024 07:01:17 -0400 Subject: [PATCH 3/5] add a comment --- tests/functional/codegen/types/test_dynamic_array.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/functional/codegen/types/test_dynamic_array.py b/tests/functional/codegen/types/test_dynamic_array.py index 17764e5237..591ec3606d 100644 --- a/tests/functional/codegen/types/test_dynamic_array.py +++ b/tests/functional/codegen/types/test_dynamic_array.py @@ -1937,6 +1937,7 @@ def foo() -> uint256: assert compile_code(code) is not None +# should not trip risky overlap detector def test_risky_call_precompile2(): code = """ x: DynArray[uint256, 32] From 652053795411d2675bd1679aafc2b3314aa0a8bd Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Thu, 20 Jun 2024 07:09:46 -0400 Subject: [PATCH 4/5] rename a property --- vyper/codegen/ir_node.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/vyper/codegen/ir_node.py b/vyper/codegen/ir_node.py index 3981b83837..be70c05226 100644 --- a/vyper/codegen/ir_node.py +++ b/vyper/codegen/ir_node.py @@ -482,12 +482,12 @@ def variable_writes(self): return ret @property - def is_call(self): + def is_call_opcode(self): return self.value in ("call", "staticcall", "delegatecall") @property def is_precompile_call(self): - assert self.is_call + assert self.is_call_opcode target = self.args[1].value lo, hi = PRECOMPILE_RANGE return isinstance(target, int) and (lo <= target <= hi) @@ -495,7 +495,7 @@ def is_precompile_call(self): @cached_property def contains_risky_call(self): ret = self.value in ("create", "create2") - ret |= self.is_call and not self.is_precompile_call + ret |= self.is_call_opcode and not self.is_precompile_call for arg in self.args: ret |= arg.contains_risky_call From 48132c972966806b0696e632908d529cc9baf627 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Thu, 20 Jun 2024 09:27:40 -0400 Subject: [PATCH 5/5] add print to safe precompiles --- vyper/codegen/ir_node.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/vyper/codegen/ir_node.py b/vyper/codegen/ir_node.py index be70c05226..0d2914b9c2 100644 --- a/vyper/codegen/ir_node.py +++ b/vyper/codegen/ir_node.py @@ -13,8 +13,6 @@ from vyper.semantics.types import VyperType from vyper.utils import VALID_IR_MACROS, ceil32 -PRECOMPILE_RANGE = (1, 10) - # Set default string representation for ints in IR output. AS_HEX_DEFAULT = False @@ -487,10 +485,18 @@ def is_call_opcode(self): @property def is_precompile_call(self): + # whitelist for "safe" calls assert self.is_call_opcode target = self.args[1].value - lo, hi = PRECOMPILE_RANGE - return isinstance(target, int) and (lo <= target <= hi) + PRECOMPILES = ( + 0x01, # builtin ecrecover + 0x02, # builtin sha256 + 0x04, # identify precompile + 0x06, # builtin ecadd + 0x07, # builtin ecmul + 0x000000000000000000636F6E736F6C652E6C6F67, # builtin print + ) + return isinstance(target, int) and target in PRECOMPILES @cached_property def contains_risky_call(self):