From 33c247151cfed13999289c08f3c35d70fa83394d Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Sun, 28 May 2023 10:33:50 -0400 Subject: [PATCH] fix: add back global calldatasize check (#3463) prevent a performance regression for sending eth to contracts with a payable default function by (mostly) reverting the changes introduced in 9ecb97b4b6f and 02339dfda0. the `skip_nonpayable_check=False` for the default function is introduced to address GHSA-vxmm-cwh2-q762 (which 02339dfda0 inadvertently fixed, and a test for which was added in 903727006c). --- .../function_definitions/external_function.py | 25 +------------------ vyper/codegen/module.py | 7 +++++- 2 files changed, 7 insertions(+), 25 deletions(-) diff --git a/vyper/codegen/function_definitions/external_function.py b/vyper/codegen/function_definitions/external_function.py index 312cb75cf8..207356860b 100644 --- a/vyper/codegen/function_definitions/external_function.py +++ b/vyper/codegen/function_definitions/external_function.py @@ -89,8 +89,7 @@ def handler_for(calldata_kwargs, default_kwargs): calldata_min_size = args_abi_t.min_size() + 4 # note we don't need the check if calldata_min_size == 4, - # because the selector checks later in this routine ensure - # that calldatasize >= 4. + # because the global calldatasize check ensures that already. if calldata_min_size > 4: ret.append(["assert", ["ge", "calldatasize", calldata_min_size]]) @@ -125,28 +124,6 @@ def handler_for(calldata_kwargs, default_kwargs): ret.append(["goto", func_t._ir_info.external_function_base_entry_label]) method_id_check = ["eq", "_calldata_method_id", method_id] - - # if there is a function whose selector is 0 or has trailing 0s, it - # might not be distinguished from the case where insufficient calldata - # is supplied, b/c calldataload loads 0s past the end of physical - # calldata (cf. yellow paper). - # since the expected behavior of supplying insufficient calldata - # is to trigger the fallback fn, we add to the selector check that - # calldatasize >= 4, which distinguishes any selector with trailing - # 0 bytes from the fallback function "selector" (equiv. to "all - # selectors not in the selector table"). - # - # note that the inclusion of this check means that, we are always - # guaranteed that the calldata is at least 4 bytes - either we have - # the explicit `calldatasize >= 4` condition in the selector check, - # or there are no trailing zeroes in the selector, (so the selector - # is impossible to match without calldatasize being at least 4). - method_id_bytes = util.method_id(abi_sig) - assert len(method_id_bytes) == 4 - has_trailing_zeroes = method_id_bytes.endswith(b"\x00") - if has_trailing_zeroes: - method_id_check = ["and", ["ge", "calldatasize", 4], method_id_check] - ret = ["if", method_id_check, ret] return ret diff --git a/vyper/codegen/module.py b/vyper/codegen/module.py index 2d498460be..64d5a8b70c 100644 --- a/vyper/codegen/module.py +++ b/vyper/codegen/module.py @@ -107,7 +107,9 @@ def _runtime_ir(runtime_functions, global_ctx): selector_section.append(func_ir) if default_function: - fallback_ir = generate_ir_for_function(default_function, global_ctx, skip_nonpayable_check) + fallback_ir = generate_ir_for_function( + default_function, global_ctx, skip_nonpayable_check=False + ) else: fallback_ir = IRnode.from_list( ["revert", 0, 0], annotation="Default function", error_msg="fallback function" @@ -119,8 +121,11 @@ def _runtime_ir(runtime_functions, global_ctx): # fallback label is the immediate next instruction, close_selector_section = ["goto", "fallback"] + global_calldatasize_check = ["if", ["lt", "calldatasize", 4], ["goto", "fallback"]] + runtime = [ "seq", + global_calldatasize_check, ["with", "_calldata_method_id", shr(224, ["calldataload", 0]), selector_section], close_selector_section, ["label", "fallback", ["var_list"], fallback_ir],