From bcd31daf944c737fd989f07950c96d2531ce807b Mon Sep 17 00:00:00 2001
From: Alex Beregszaszi <alex@rtfs.hu>
Date: Wed, 28 Oct 2020 14:31:49 +0000
Subject: [PATCH] [ewasm] Terminate on out-of-bounds access in EwasmInterpreter

---
 .../EwasmBuiltinInterpreter.cpp               | 85 +++++++++----------
 .../yulInterpreter/EwasmBuiltinInterpreter.h  |  3 +-
 2 files changed, 41 insertions(+), 47 deletions(-)

diff --git a/test/tools/yulInterpreter/EwasmBuiltinInterpreter.cpp b/test/tools/yulInterpreter/EwasmBuiltinInterpreter.cpp
index 6d3f3a5f4..10c8af230 100644
--- a/test/tools/yulInterpreter/EwasmBuiltinInterpreter.cpp
+++ b/test/tools/yulInterpreter/EwasmBuiltinInterpreter.cpp
@@ -115,8 +115,6 @@ uint64_t popcnt(uint64_t _v)
 
 }
 
-using u512 = boost::multiprecision::number<boost::multiprecision::cpp_int_backend<512, 256, boost::multiprecision::unsigned_magnitude, boost::multiprecision::unchecked, void>>;
-
 u256 EwasmBuiltinInterpreter::evalBuiltin(
 	YulString _functionName,
 	vector<Expression> const& _arguments,
@@ -144,14 +142,14 @@ u256 EwasmBuiltinInterpreter::evalBuiltin(
 	else if (fun == "datacopy")
 	{
 		// This is identical to codecopy.
-		if (accessMemory(_evaluatedArguments.at(0), _evaluatedArguments.at(2)))
-			copyZeroExtended(
-				m_state.memory,
-				m_state.code,
-				static_cast<size_t>(_evaluatedArguments.at(0)),
-				static_cast<size_t>(_evaluatedArguments.at(1) & numeric_limits<size_t>::max()),
-				static_cast<size_t>(_evaluatedArguments.at(2))
-			);
+		accessMemory(_evaluatedArguments.at(0), _evaluatedArguments.at(2));
+		copyZeroExtended(
+			m_state.memory,
+			m_state.code,
+			static_cast<size_t>(_evaluatedArguments.at(0)),
+			static_cast<size_t>(_evaluatedArguments.at(1) & numeric_limits<size_t>::max()),
+			static_cast<size_t>(_evaluatedArguments.at(2))
+		);
 		return 0;
 	}
 	else if (fun == "i32.drop" || fun == "i64.drop" || fun == "nop")
@@ -324,11 +322,11 @@ u256 EwasmBuiltinInterpreter::evalEthBuiltin(string const& _fun, vector<uint64_t
 	{
 		if (arg[1] + arg[2] < arg[1] || arg[1] + arg[2] > m_state.calldata.size())
 			throw ExplicitlyTerminated();
-		if (accessMemory(arg[0], arg[2]))
-			copyZeroExtended(
-				m_state.memory, m_state.calldata,
-				size_t(arg[0]), size_t(arg[1]), size_t(arg[2])
-			);
+		accessMemory(arg[0], arg[2]);
+		copyZeroExtended(
+			m_state.memory, m_state.calldata,
+			size_t(arg[0]), size_t(arg[1]), size_t(arg[2])
+		);
 		return {};
 	}
 	else if (_fun == "getCallDataSize")
@@ -377,11 +375,11 @@ u256 EwasmBuiltinInterpreter::evalEthBuiltin(string const& _fun, vector<uint64_t
 	}
 	else if (_fun == "codeCopy")
 	{
-		if (accessMemory(arg[0], arg[2]))
-			copyZeroExtended(
-				m_state.memory, m_state.code,
-				size_t(arg[0]), size_t(arg[1]), size_t(arg[2])
-			);
+		accessMemory(arg[0], arg[2]);
+		copyZeroExtended(
+			m_state.memory, m_state.code,
+			size_t(arg[0]), size_t(arg[1]), size_t(arg[2])
+		);
 		return 0;
 	}
 	else if (_fun == "getCodeSize")
@@ -407,12 +405,12 @@ u256 EwasmBuiltinInterpreter::evalEthBuiltin(string const& _fun, vector<uint64_t
 	else if (_fun == "externalCodeCopy")
 	{
 		readAddress(arg[0]);
-		if (accessMemory(arg[1], arg[3]))
-			// TODO this way extcodecopy and codecopy do the same thing.
-			copyZeroExtended(
-				m_state.memory, m_state.code,
-				size_t(arg[1]), size_t(arg[2]), size_t(arg[3])
-			);
+		accessMemory(arg[1], arg[3]);
+		// TODO this way extcodecopy and codecopy do the same thing.
+		copyZeroExtended(
+			m_state.memory, m_state.code,
+			size_t(arg[1]), size_t(arg[2]), size_t(arg[3])
+		);
 		return 0;
 	}
 	else if (_fun == "getExternalCodeSize")
@@ -454,16 +452,16 @@ u256 EwasmBuiltinInterpreter::evalEthBuiltin(string const& _fun, vector<uint64_t
 	else if (_fun == "finish")
 	{
 		bytes data;
-		if (accessMemory(arg[0], arg[1]))
-			data = readMemory(arg[0], arg[1]);
+		accessMemory(arg[0], arg[1]);
+		data = readMemory(arg[0], arg[1]);
 		logTrace(evmasm::Instruction::RETURN, {}, data);
 		throw ExplicitlyTerminated();
 	}
 	else if (_fun == "revert")
 	{
 		bytes data;
-		if (accessMemory(arg[0], arg[1]))
-			data = readMemory(arg[0], arg[1]);
+		accessMemory(arg[0], arg[1]);
+		data = readMemory(arg[0], arg[1]);
 		logTrace(evmasm::Instruction::REVERT, {}, data);
 		throw ExplicitlyTerminated();
 	}
@@ -473,11 +471,11 @@ u256 EwasmBuiltinInterpreter::evalEthBuiltin(string const& _fun, vector<uint64_t
 	{
 		if (arg[1] + arg[2] < arg[1] || arg[1] + arg[2] > m_state.returndata.size())
 			throw ExplicitlyTerminated();
-		if (accessMemory(arg[0], arg[2]))
-			copyZeroExtended(
-				m_state.memory, m_state.calldata,
-				size_t(arg[0]), size_t(arg[1]), size_t(arg[2])
-			);
+		accessMemory(arg[0], arg[2]);
+		copyZeroExtended(
+			m_state.memory, m_state.calldata,
+			size_t(arg[0]), size_t(arg[1]), size_t(arg[2])
+		);
 		return {};
 	}
 	else if (_fun == "selfDestruct")
@@ -494,18 +492,15 @@ u256 EwasmBuiltinInterpreter::evalEthBuiltin(string const& _fun, vector<uint64_t
 	return 0;
 }
 
-bool EwasmBuiltinInterpreter::accessMemory(u256 const& _offset, u256 const& _size)
+void EwasmBuiltinInterpreter::accessMemory(u256 const& _offset, u256 const& _size)
 {
-	if (((_offset + _size) >= _offset) && ((_offset + _size + 0x1f) >= (_offset + _size)))
-	{
-		u256 newSize = (_offset + _size + 0x1f) & ~u256(0x1f);
-		m_state.msize = max(m_state.msize, newSize);
-		return _size <= 0xffff;
-	}
-	else
-		m_state.msize = u256(-1);
+	// Single WebAssembly page.
+	// TODO: Support expansion in this interpreter.
+	m_state.msize = 65536;
 
-	return false;
+	if (((_offset + _size) < _offset) || ((_offset + _size) > m_state.msize))
+		// Ewasm throws out of bounds exception as opposed to the EVM.
+		throw ExplicitlyTerminated();
 }
 
 bytes EwasmBuiltinInterpreter::readMemory(uint64_t _offset, uint64_t _size)
diff --git a/test/tools/yulInterpreter/EwasmBuiltinInterpreter.h b/test/tools/yulInterpreter/EwasmBuiltinInterpreter.h
index 44037a083..24e85cf2a 100644
--- a/test/tools/yulInterpreter/EwasmBuiltinInterpreter.h
+++ b/test/tools/yulInterpreter/EwasmBuiltinInterpreter.h
@@ -91,8 +91,7 @@ private:
 
 	/// Checks if the memory access is not too large for the interpreter and adjusts
 	/// msize accordingly.
-	/// @returns false if the amount of bytes read is lager than 0xffff
-	bool accessMemory(u256 const& _offset, u256 const& _size = 32);
+	void 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
 	bytes readMemory(uint64_t _offset, uint64_t _size = 32);