fix: calldatasize < 4 reverting instead of going to fallback

tests/parser/functions/test_default_function.py

@@ -100,7 +100,9 @@ def __default__():
     assert_compile_failed(lambda: get_contract_with_gas_estimation(code))
 
 
-def test_zero_method_id(w3, get_logs, get_contract_with_gas_estimation):
+def test_zero_method_id(w3, get_logs, get_contract, assert_tx_failed):
+    # test a method with 0x00000000 selector,
+    # expects at least 36 bytes of calldata.
     code = """
 event Sent:
     sig: uint256
@@ -116,18 +118,108 @@ def blockHashAskewLimitary(v: uint256) -> uint256:
 def __default__():
     log Sent(1)
     """
-
-    c = get_contract_with_gas_estimation(code)
+    c = get_contract(code)
 
     assert c.blockHashAskewLimitary(0) == 7
 
-    logs = get_logs(w3.eth.send_transaction({"to": c.address, "value": 0}), c, "Sent")
-    assert 1 == logs[0].args.sig
+    def _call_with_bytes(hexstr):
+        # call our special contract and return the logged value
+        logs = get_logs(
+            w3.eth.send_transaction({"to": c.address, "value": 0, "data": hexstr}), c, "Sent"
+        )
+        return logs[0].args.sig
 
-    logs = get_logs(
-        # call blockHashAskewLimitary
-        w3.eth.send_transaction({"to": c.address, "value": 0, "data": "0x" + "00" * 36}),
-        c,
-        "Sent",
-    )
-    assert 2 == logs[0].args.sig
+    assert 1 == _call_with_bytes("0x")
+
+    # call blockHashAskewLimitary with proper calldata
+    assert 2 == _call_with_bytes("0x" + "00" * 36)
+
+    # call blockHashAskewLimitary with extra trailing bytes in calldata
+    assert 2 == _call_with_bytes("0x" + "00" * 37)
+
+    for i in range(4):
+        # less than 4 bytes of calldata doesn't match the 0 selector and goes to default
+        assert 1 == _call_with_bytes("0x" + "00" * i)
+
+    for i in range(4, 36):
+        # match the full 4 selector bytes, but revert due to malformed (short) calldata
+        assert_tx_failed(lambda: _call_with_bytes("0x" + "00" * i))
+
+
+def test_another_zero_method_id(w3, get_logs, get_contract, assert_tx_failed):
+    # test another zero method id but which only expects 4 bytes of calldata
+    code = """
+event Sent:
+    sig: uint256
+
+@external
+@payable
+# function selector: 0x00000000
+def wycpnbqcyf() -> uint256:
+    log Sent(2)
+    return 7
+
+@external
+def __default__():
+    log Sent(1)
+    """
+    c = get_contract(code)
+
+    assert c.wycpnbqcyf() == 7
+
+    def _call_with_bytes(hexstr):
+        # call our special contract and return the logged value
+        logs = get_logs(
+            w3.eth.send_transaction({"to": c.address, "value": 0, "data": hexstr}), c, "Sent"
+        )
+        return logs[0].args.sig
+
+    assert 1 == _call_with_bytes("0x")
+
+    # call wycpnbqcyf
+    assert 2 == _call_with_bytes("0x" + "00" * 4)
+
+    # too many bytes ok
+    assert 2 == _call_with_bytes("0x" + "00" * 5)
+
+    # "right" method id but by accident - not enough bytes.
+    for i in range(4):
+        assert 1 == _call_with_bytes("0x" + "00" * i)
+
+
+def test_partial_selector_match_trailing_zeroes(w3, get_logs, get_contract):
+    code = """
+event Sent:
+    sig: uint256
+
+@external
+@payable
+# function selector: 0xd88e0b00
+def fow() -> uint256:
+    log Sent(2)
+    return 7
+
+@external
+def __default__():
+    log Sent(1)
+    """
+    c = get_contract(code)
+
+    # sanity check - we can call c.fow()
+    assert c.fow() == 7
+
+    def _call_with_bytes(hexstr):
+        # call our special contract and return the logged value
+        logs = get_logs(
+            w3.eth.send_transaction({"to": c.address, "value": 0, "data": hexstr}), c, "Sent"
+        )
+        return logs[0].args.sig
+
+    # check we can call default function
+    assert 1 == _call_with_bytes("0x")
+
+    # check fow() selector is 0xd88e0b00
+    assert 2 == _call_with_bytes("0xd88e0b00")
+
+    # check calling d88e0b with no trailing zero goes to fallback instead of reverting
+    assert 1 == _call_with_bytes("0xd88e0b")

vyper/codegen/function_definitions/external_function.py

@@ -88,7 +88,12 @@ def handler_for(calldata_kwargs, default_kwargs):
         # ensure calldata is at least of minimum length
         args_abi_t = calldata_args_t.abi_type
         calldata_min_size = args_abi_t.min_size() + 4
-        ret.append(["assert", ["ge", "calldatasize", calldata_min_size]])
+
+        # note we don't need the check if calldata_min_size == 4,
+        # because the selector checks later in this routine ensure
+        # that calldatasize >= 4.
+        if calldata_min_size > 4:
+            ret.append(["assert", ["ge", "calldatasize", calldata_min_size]])
 
         # TODO optimize make_setter by using
         # TupleT(list(arg.typ for arg in calldata_kwargs + default_kwargs))
@@ -124,20 +129,26 @@ def handler_for(calldata_kwargs, default_kwargs):
 
         method_id_check = ["eq", "_calldata_method_id", method_id]
 
-        # if there is a function whose selector is 0, it won't be distinguished
-        # from the case where nil calldata is supplied, b/c calldataload loads
-        # 0s past the end of physical calldata (cf. yellow paper).
-        # since supplying 0 calldata is expected to trigger the fallback fn,
-        # we check that calldatasize > 0, which distinguishes the 0 selector
-        # from the fallback function "selector"
-        # (equiv. to "all selectors not in the selector table").
-
-        # note: cases where not enough calldata is supplied (besides
-        # calldatasize==0) are not addressed here b/c a calldatasize
-        # well-formedness check is already present in the function body
-        # as part of abi validation
-        if method_id.value == 0:
-            method_id_check = ["and", ["gt", "calldatasize", 0], method_id_check]
+        # 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