Merge pull request from GHSA-mgv8-gggw-mrg6

* fix: block storage allocator overflows

the storage allocator did not guard against overflow when no storage
layout override was provided. this could result in vulnerabilities like
the following:

```vyper
owner: public(address)
buffer: public(uint256[max_value(uint256)])

@external
def initialize():
    self.owner = msg.sender

@external
def foo(idx: uint256, data: uint256):
    self.buffer[idx] = data
```

while the get_element_ptr calculation for `self.buffer[idx]` is checked,
it is not checked in `mod_{2**256}` arithmetic, which can lead to
arithmetic wrapping back to the `owner` variable if the provided `idx`
is large enough.

* clean up allocator logic

also fix a bug where large allocations would use too much storage due to
floating point rounding precision

* add warning for large arrays

* add note about 2**64 behavior

docs/types.rst

@@ -520,6 +520,9 @@ A two dimensional list can be declared with ``_name: _ValueType[inner_size][oute
     # Returning the value in row 0 column 4 (in this case 14)
     return exampleList2D[0][4]
 
+.. note::
+    Defining an array in storage whose size is significantly larger than ``2**64`` can result in security vulnerabilities due to risk of overflow.
+
 .. index:: !dynarrays
 
 Dynamic Arrays
@@ -561,6 +564,10 @@ Dynamic arrays represent bounded arrays whose length can be modified at runtime,
 
 In the ABI, they are represented as ``_Type[]``. For instance, ``DynArray[int128, 3]`` gets represented as ``int128[]``, and ``DynArray[DynArray[int128, 3], 3]`` gets represented as ``int128[][]``.
 
+.. note::
+    Defining a dynamic array in storage whose size is significantly larger than ``2**64`` can result in security vulnerabilities due to risk of overflow.
+
+
 .. _types-struct:
 
 Structs

tests/cli/outputs/test_storage_layout_overrides.py

@@ -95,6 +95,21 @@ def test_simple_collision():
         )
 
 
+def test_overflow():
+    code = """
+x: uint256[2]
+    """
+
+    storage_layout_override = {"x": {"slot": 2**256 - 1, "type": "uint256[2]"}}
+
+    with pytest.raises(
+        StorageLayoutException, match=f"Invalid storage slot for var x, out of bounds: {2**256}\n"
+    ):
+        compile_code(
+            code, output_formats=["layout"], storage_layout_override=storage_layout_override
+        )
+
+
 def test_incomplete_overrides():
     code = """
 name: public(String[64])

tests/functional/test_storage_slots.py

@@ -1,3 +1,7 @@
+import pytest
+
+from vyper.exceptions import StorageLayoutException
+
 code = """
 
 struct StructOne:
@@ -97,3 +101,15 @@ def test_reentrancy_lock(get_contract):
     assert [c.foo(0, i) for i in range(3)] == [987, 654, 321]
     assert [c.foo(1, i) for i in range(3)] == [123, 456, 789]
     assert c.h(0) == 123456789
+
+
+def test_allocator_overflow(get_contract):
+    code = """
+x: uint256
+y: uint256[max_value(uint256)]
+    """
+    with pytest.raises(
+        StorageLayoutException,
+        match=f"Invalid storage slot for var y, tried to allocate slots 1 through {2**256}\n",
+    ):
+        get_contract(code)

vyper/semantics/analysis/data_positions.py

@@ -6,6 +6,7 @@
 from vyper.exceptions import StorageLayoutException
 from vyper.semantics.analysis.base import CodeOffset, StorageSlot
 from vyper.typing import StorageLayout
+from vyper.utils import ceil32
 
 
 def set_data_positions(
@@ -121,8 +122,7 @@ def set_storage_slots_with_overrides(
         # Expect to find this variable within the storage layout overrides
         if node.target.id in storage_layout_overrides:
             var_slot = storage_layout_overrides[node.target.id]["slot"]
-            # Calculate how many storage slots are required
-            storage_length = math.ceil(varinfo.typ.size_in_bytes / 32)
+            storage_length = varinfo.typ.storage_size_in_words
             # Ensure that all required storage slots are reserved, and prevents other variables
             # from using these slots
             reserved_slots.reserve_slot_range(var_slot, storage_length, node.target.id)
@@ -139,14 +139,29 @@ def set_storage_slots_with_overrides(
     return ret
 
 
+class SimpleStorageAllocator:
+    def __init__(self, starting_slot: int = 0):
+        self._slot = starting_slot
+
+    def allocate_slot(self, n, var_name):
+        ret = self._slot
+        if self._slot + n >= 2**256:
+            raise StorageLayoutException(
+                f"Invalid storage slot for var {var_name}, tried to allocate"
+                f" slots {self._slot} through {self._slot + n}"
+            )
+        self._slot += n
+        return ret
+
+
 def set_storage_slots(vyper_module: vy_ast.Module) -> StorageLayout:
     """
     Parse module-level Vyper AST to calculate the layout of storage variables.
     Returns the layout as a dict of variable name -> variable info
     """
     # Allocate storage slots from 0
     # note storage is word-addressable, not byte-addressable
-    storage_slot = 0
+    allocator = SimpleStorageAllocator()
 
     ret: Dict[str, Dict] = {}
 
@@ -165,36 +180,38 @@ def set_storage_slots(vyper_module: vy_ast.Module) -> StorageLayout:
             type_.set_reentrancy_key_position(StorageSlot(_slot))
             continue
 
-        type_.set_reentrancy_key_position(StorageSlot(storage_slot))
+        # TODO use one byte - or bit - per reentrancy key
+        # requires either an extra SLOAD or caching the value of the
+        # location in memory at entrance
+        slot = allocator.allocate_slot(1, variable_name)
+
+        type_.set_reentrancy_key_position(StorageSlot(slot))
 
         # TODO this could have better typing but leave it untyped until
         # we nail down the format better
-        ret[variable_name] = {"type": "nonreentrant lock", "slot": storage_slot}
+        ret[variable_name] = {"type": "nonreentrant lock", "slot": slot}
 
-        # TODO use one byte - or bit - per reentrancy key
-        # requires either an extra SLOAD or caching the value of the
-        # location in memory at entrance
-        storage_slot += 1
 
     for node in vyper_module.get_children(vy_ast.VariableDecl):
         # skip non-storage variables
         if node.is_constant or node.is_immutable:
             continue
 
         varinfo = node.target._metadata["varinfo"]
-        varinfo.set_position(StorageSlot(storage_slot))
-
         type_ = varinfo.typ
 
-        # this could have better typing but leave it untyped until
-        # we understand the use case better
-        ret[node.target.id] = {"type": str(type_), "slot": storage_slot}
-
         # CMC 2021-07-23 note that HashMaps get assigned a slot here.
         # I'm not sure if it's safe to avoid allocating that slot
         # for HashMaps because downstream code might use the slot
         # ID as a salt.
-        storage_slot += math.ceil(type_.size_in_bytes / 32)
+        n_slots = type_.storage_size_in_words
+        slot = allocator.allocate_slot(n_slots, node.target.id)
+
+        varinfo.set_position(StorageSlot(slot))
+
+        # this could have better typing but leave it untyped until
+        # we understand the use case better
+        ret[node.target.id] = {"type": str(type_), "slot": slot}
 
     return ret
 
@@ -216,7 +233,7 @@ def set_code_offsets(vyper_module: vy_ast.Module) -> Dict:
         type_ = varinfo.typ
         varinfo.set_position(CodeOffset(offset))
 
-        len_ = math.ceil(type_.size_in_bytes / 32) * 32
+        len_ = ceil32(type_.size_in_bytes)
 
         # this could have better typing but leave it untyped until
         # we understand the use case better

vyper/semantics/types/subscriptable.py

@@ -103,6 +103,9 @@ def __init__(self, value_type: VyperType, length: int):
         if not 0 < length < 2**256:
             raise InvalidType("Array length is invalid")
 
+        if length >= 2**64:
+            warnings.warn("Use of large arrays can be unsafe!")
+
         super().__init__(UINT256_T, value_type)
         self.length = length