Merge pull request #13516 from ethereum/fix-yul-interpreter-memory-access

Fix memory expansion semantics in the yul interpreter.

test/libyul/yulInterpreterTests/zero_range.yul

@@ -0,0 +1,10 @@
+{
+  calldatacopy(32, 0, 0)
+  // Used to store 32
+  sstore(0, msize())
+}
+// ----
+// Trace:
+//   CALLDATACOPY(32, 0, 0)
+// Memory dump:
+// Storage dump:

test/tools/yulInterpreter/EVMInstructionInterpreter.cpp

@@ -499,13 +499,15 @@ u256 EVMInstructionInterpreter::evalBuiltin(
 
 bool EVMInstructionInterpreter::accessMemory(u256 const& _offset, u256 const& _size)
 {
-	if (((_offset + _size) >= _offset) && ((_offset + _size + 0x1f) >= (_offset + _size)))
+	if (_size == 0)
+		return true;
+	else if (((_offset + _size) >= _offset) && ((_offset + _size + 0x1f) >= (_offset + _size)))
 	{
 		u256 newSize = (_offset + _size + 0x1f) & ~u256(0x1f);
 		m_state.msize = max(m_state.msize, newSize);
-		// We only record accesses to contiguous memory chunks that are at most 0xffff bytes
+		// We only record accesses to contiguous memory chunks that are at most s_maxRangeSize bytes
-		// in size and at an offset of at most numeric_limits<size_t>::max() - 0xffff
+		// in size and at an offset of at most numeric_limits<size_t>::max() - s_maxRangeSize
-		return _size <= 0xffff && _offset <= u256(numeric_limits<size_t>::max() - 0xffff);
+		return _size <= s_maxRangeSize && _offset <= u256(numeric_limits<size_t>::max() - s_maxRangeSize);
 	}
 	else
 		m_state.msize = u256(-1);
@@ -513,6 +515,15 @@ bool EVMInstructionInterpreter::accessMemory(u256 const& _offset, u256 const& _s
 	return false;
 }
 
+bytes EVMInstructionInterpreter::readMemory(u256 const& _offset, u256 const& _size)
+{
+	yulAssert(_size <= s_maxRangeSize, "Too large read.");
+	bytes data(size_t(_size), uint8_t(0));
+	for (size_t i = 0; i < data.size(); ++i)
+		data[i] = m_state.memory[_offset + i];
+	return data;
+}
+
 u256 EVMInstructionInterpreter::readMemoryWord(u256 const& _offset)
 {
 	return u256(h256(m_state.readMemory(_offset, 32)));

test/tools/yulInterpreter/EVMInstructionInterpreter.h

@@ -88,9 +88,15 @@ public:
 	);
 
 private:
-	/// Checks if the memory access is not too large for the interpreter and adjusts
+	/// Checks if the memory access is valid and adjusts msize accordingly.
-	/// msize accordingly.
+	/// @returns true if memory access is valid, false otherwise
-	/// @returns false if the amount of bytes read is lager than 0xffff
+	/// A valid memory access must satisfy all of the following pre-requisites:
+	/// - Sum of @param _offset and @param _size do not overflow modulo u256
+	/// - Sum of @param _offset, @param _size, and 31 do not overflow modulo u256 (see note below)
+	/// - @param _size is lesser than or equal to @a s_maxRangeSize
+	/// - @param _offset is lesser than or equal to the difference of numeric_limits<size_t>::max()
+	/// and @a s_maxRangeSize
+	/// Note: Memory expansion is carried out in multiples of 32 bytes.
 	bool accessMemory(u256 const& _offset, u256 const& _size = 32);
 	/// @returns the memory contents at the provided address.
 	/// Does not adjust msize, use @a accessMemory for that
@@ -125,6 +131,9 @@ private:
 	InterpreterState& m_state;
 	/// Flag to disable trace of instructions that write to memory.
 	bool m_disableMemoryWriteInstructions;
+public:
+	/// Maximum length for range-based memory access operations.
+	static constexpr unsigned s_maxRangeSize = 0xffff;
 };
 
 } // solidity::yul::test