diff --git a/docs/types.rst b/docs/types.rst index 2c3a1e75d8..d669e6946d 100644 --- a/docs/types.rst +++ b/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 diff --git a/tests/cli/outputs/test_storage_layout_overrides.py b/tests/cli/outputs/test_storage_layout_overrides.py index ff86b0cdf1..94e0faeb37 100644 --- a/tests/cli/outputs/test_storage_layout_overrides.py +++ b/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]) diff --git a/tests/functional/test_storage_slots.py b/tests/functional/test_storage_slots.py index 4653625468..d390fe9a39 100644 --- a/tests/functional/test_storage_slots.py +++ b/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) diff --git a/vyper/semantics/analysis/data_positions.py b/vyper/semantics/analysis/data_positions.py index 5c1347d5a3..8270f14558 100644 --- a/vyper/semantics/analysis/data_positions.py +++ b/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,6 +139,21 @@ 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. @@ -146,7 +161,7 @@ def set_storage_slots(vyper_module: vy_ast.Module) -> StorageLayout: """ # Allocate storage slots from 0 # note storage is word-addressable, not byte-addressable - storage_slot = 0 + allocator = SimpleStorageAllocator() ret: Dict[str, Dict] = {} @@ -165,16 +180,17 @@ 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 @@ -182,19 +198,20 @@ def set_storage_slots(vyper_module: vy_ast.Module) -> StorageLayout: 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 diff --git a/vyper/semantics/types/subscriptable.py b/vyper/semantics/types/subscriptable.py index a5ae075b73..4aef58ca23 100644 --- a/vyper/semantics/types/subscriptable.py +++ b/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