Merge pull request #30 from cyberthirst/fix/slice-overflow

Fix/slice overflow

tests/functional/builtins/codegen/test_slice.py

@@ -435,3 +435,66 @@ def test_slice_bytes32_calldata_extended(get_contract, code, result):
         c.bar(3, "0x0001020304050607080910111213141516171819202122232425262728293031", 5).hex()
         == result
     )
+
+
+# test cases crafted based on advisory GHSA-9x7f-gwxq-6f2c
+oob_fail_list = [
+    """
+d: public(Bytes[256])
+	
+@external
+def do_slice():
+	x : uint256 = max_value(uint256) 
+	self.d = b"\x01\x02\x03\x04\x05\x06"
+	assert len(slice(self.d, 1, x)) == max_value(uint256)
+    """,
+    """
+@external
+def do_slice():
+    x: uint256 = max_value(uint256)
+    # y == 0x3232323232323232323232323232323232323232323232323232323232323232
+    y: uint256 = 22704331223003175573249212746801550559464702875615796870481879217237868556850
+    z: uint96 = 1
+    if True:
+        placeholder : uint256[16] = [y, y, y, y, y, y, y, y, y, y, y, y, y, y, y, y]
+    s :String[32] = slice(uint2str(z), 1, x)
+    assert slice(s, 1, 2) == "22"
+    """,
+    """
+x: public(Bytes[64])
+secret: uint256
+
+@deploy
+def __init__():
+    self.x = empty(Bytes[64])
+    self.secret = 42
+
+@external
+def do_slice() -> Bytes[64]:
+    start: uint256 = max_value(uint256) - 63
+    return slice(self.x, start, 64) 
+    """,
+    # tests bounds check in adhoc location calldata
+    """
+interface IFace:
+    def choose_value(_x: uint256, _y: uint256, _z: uint256, idx: uint256) -> Bytes[32]: nonpayable
+
+@external
+def choose_value(_x: uint256, _y: uint256, _z: uint256, idx: uint256) -> Bytes[32]:
+    assert idx % 32 == 4
+    return slice(msg.data, idx, 32)
+
+@external
+def do_slice():
+    idx: uint256 = max_value(uint256) - 27
+    ret: uint256 = _abi_decode(extcall IFace(self).choose_value(1, 2, 3, idx), uint256)
+    assert ret == 0
+    """
+]
+
+
+@pytest.mark.parametrize("bad_code", oob_fail_list)
+def test_slice_buffer_oob_reverts(bad_code, get_contract, tx_failed):
+    c = get_contract(bad_code)
+    with tx_failed():
+        c.do_slice()

vyper/builtins/functions.py

@@ -229,6 +229,17 @@ def build_IR(self, expr, context):
 ADHOC_SLICE_NODE_MACROS = ["~calldata", "~selfcode", "~extcode"]
 
 
+# make sure we don't overrun the source buffer, checking for
+# overflow:
+# valid inputs satisfy: `assert start+length <= src_len && start+length > start`
+def _make_slice_bounds_check(start, length, src_len):
+    return [
+        "with",
+        "end",
+        ["add", start, length],
+        ["assert", ["iszero", ["or", ["gt", "end", src_len], ["lt", "end", start]]]],
+    ]
+
 def _build_adhoc_slice_node(sub: IRnode, start: IRnode, length: IRnode, context: Context) -> IRnode:
     assert length.is_literal, "typechecker failed"
     assert isinstance(length.value, int)  # mypy hint
@@ -241,7 +252,7 @@ def _build_adhoc_slice_node(sub: IRnode, start: IRnode, length: IRnode, context:
     if sub.value == "~calldata":
         node = [
             "seq",
-            ["assert", ["le", ["add", start, length], "calldatasize"]],  # runtime bounds check
+            _make_slice_bounds_check(start, length, "calldatasize"),
             ["mstore", np, length],
             ["calldatacopy", np + 32, start, length],
             np,
@@ -251,7 +262,7 @@ def _build_adhoc_slice_node(sub: IRnode, start: IRnode, length: IRnode, context:
     elif sub.value == "~selfcode":
         node = [
             "seq",
-            ["assert", ["le", ["add", start, length], "codesize"]],  # runtime bounds check
+            _make_slice_bounds_check(start, length, "codesize"),
             ["mstore", np, length],
             ["codecopy", np + 32, start, length],
             np,
@@ -266,8 +277,7 @@ def _build_adhoc_slice_node(sub: IRnode, start: IRnode, length: IRnode, context:
             sub.args[0],
             [
                 "seq",
-                # runtime bounds check
-                ["assert", ["le", ["add", start, length], ["extcodesize", "_extcode_address"]]],
+                _make_slice_bounds_check(start, length, ["extcodesize", "_extcode_address"]),
                 ["mstore", np, length],
                 ["extcodecopy", "_extcode_address", np + 32, start, length],
                 np,
@@ -438,19 +448,9 @@ def build_IR(self, expr, args, kwargs, context):
 
             do_copy = copy_bytes(copy_dst, copy_src, copy_len, copy_maxlen)
 
-            # make sure we don't overrun the source buffer, checking for
-            # overflow:
-            # `assert start+length <= src_len && start+length <= length`
-            bounds_check = [
-                "with",
-                "end",
-                ["add", start, length],
-                ["assert", ["iszero", ["or", ["gt", "end", src_len], ["gt", "end", length]]]],
-            ]
-
             ret = [
                 "seq",
-                bounds_check,
+                _make_slice_bounds_check(start, length, src_len),
                 do_copy,
                 ["mstore", dst, length],  # set length
                 dst,  # return pointer to dst