Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/better-tires-pull.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'openzeppelin-solidity': minor
---

`Memory`: Add a `isReserved(Slice)` function.
5 changes: 5 additions & 0 deletions .changeset/gentle-apples-dance.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'openzeppelin-solidity': minor
---

`Accumulator`: Check that slices being added (shift or push) are in the reserved space.
9 changes: 9 additions & 0 deletions contracts/utils/Memory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,15 @@ library Memory {
}
}

/// @dev Returns true if the memory occupied by the slice is reserved.
function isReserved(Slice self) internal pure returns (bool result) {
Memory.Pointer fmp = getFreeMemoryPointer();
Memory.Pointer end = forward(_pointer(self), length(self));
assembly ("memory-safe") {
result := iszero(lt(fmp, end)) // end <= fmp
}
}

/**
* @dev Private helper: create a slice from raw values (length and pointer)
*
Expand Down
5 changes: 5 additions & 0 deletions contracts/utils/structs/Accumulators.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
pragma solidity ^0.8.24;

import {Memory} from "../Memory.sol";
import {Panic} from "../Panic.sol";

/**
* @dev Structure concatenating an arbitrary number of bytes buffers with limited memory allocation.
Expand Down Expand Up @@ -59,6 +60,8 @@ library Accumulators {

/// @dev Add a memory slice to (the end of) an Accumulator
function push(Accumulator memory self, Memory.Slice data) internal pure returns (Accumulator memory) {
if (!data.isReserved()) Panic.panic(Panic.RESOURCE_ERROR);

Memory.Pointer ptr = _asPtr(AccumulatorEntry({next: _nullPtr(), data: data}));

if (_nullPtr().equal(self.head)) {
Expand All @@ -79,6 +82,8 @@ library Accumulators {

/// @dev Add a memory slice to (the beginning of) an Accumulator
function shift(Accumulator memory self, Memory.Slice data) internal pure returns (Accumulator memory) {
if (!data.isReserved()) Panic.panic(Panic.RESOURCE_ERROR);

Memory.Pointer ptr = _asPtr(AccumulatorEntry({next: self.head, data: data}));

if (_nullPtr().equal(self.head)) {
Expand Down
40 changes: 37 additions & 3 deletions test/utils/Memory.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,52 @@ contract MemoryTest is Test {
}

function testAsSliceToBytes(bytes memory input) public pure {
assertEq(input.asSlice().toBytes(), input);
Memory.Slice slice = input.asSlice();
assertEq(slice.toBytes(), input);
assertTrue(slice.isReserved());
}

function testSlice(bytes memory input, uint256 offset) public pure {
offset = bound(offset, 0, input.length);
assertEq(input.asSlice().slice(offset).toBytes(), input.slice(offset));

Memory.Slice slice = input.asSlice().slice(offset);
assertEq(slice.toBytes(), input.slice(offset));
assertTrue(slice.isReserved());
}

function testSlice(bytes memory input, uint256 offset, uint256 length) public pure {
offset = bound(offset, 0, input.length);
length = bound(length, 0, input.length - offset);
assertEq(input.asSlice().slice(offset, length).toBytes(), input.slice(offset, offset + length));

Memory.Slice slice = input.asSlice().slice(offset, length);
assertEq(slice.toBytes(), input.slice(offset, offset + length));
assertTrue(slice.isReserved());
}

function testInvalidSlice1() public pure {
bytes memory input = new bytes(256);
Comment on lines +47 to +48
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe?

Suggested change
function testInvalidSlice1() public pure {
bytes memory input = new bytes(256);
function testInvalidSliceOutOfBoundEnd(bytes memory input) public pure {

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will not work. For input objects that have a length that are not a multiple of 32, solidity will set the FMP to a multiple of 32. This means adding 1 is not enough to guarantee the error (because there is more reserved memory than strictly needed)

Memory.Slice slice = input.asSlice();

assertTrue(slice.isReserved());

assembly ("memory-safe") {
slice := add(slice, 0x01) // add 1 to the ptr part
}

assertFalse(slice.isReserved());
}

function testInvalidSlice2() public pure {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
function testInvalidSlice2() public pure {
function testInvalidSliceOutOfBoundLength() public pure {

bytes memory input = new bytes(256);
Comment on lines +60 to +61
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
function testInvalidSlice2() public pure {
bytes memory input = new bytes(256);
function testInvalidSlice2(bytes memory input) public pure {

Memory.Slice slice = input.asSlice();

assertTrue(slice.isReserved());

assembly ("memory-safe") {
slice := add(slice, shl(128, 0x01)) // add 1 to the length part
}

assertFalse(slice.isReserved());
}

function testSymbolicEqual(bytes memory a, bytes memory b) public pure {
Expand Down