From 034d436933806c204b84b0500e2116938cb6a030 Mon Sep 17 00:00:00 2001
From: chriseth <c@ethdev.com>
Date: Sun, 7 Aug 2016 19:40:19 +0200
Subject: [PATCH] Make ecrecover throw for malformed input.

---
 libsolidity/codegen/ExpressionCompiler.cpp | 45 +++++++++++++++++++---
 test/libsolidity/SolidityEndToEndTest.cpp  | 18 +++++++++
 2 files changed, 58 insertions(+), 5 deletions(-)

diff --git a/libsolidity/codegen/ExpressionCompiler.cpp b/libsolidity/codegen/ExpressionCompiler.cpp
index 1d5745560..50148901c 100644
--- a/libsolidity/codegen/ExpressionCompiler.cpp
+++ b/libsolidity/codegen/ExpressionCompiler.cpp
@@ -1445,6 +1445,19 @@ void ExpressionCompiler::appendExternalFunctionCall(
 		argumentTypes.push_back(_arguments[i]->annotation().type);
 	}
 
+	if (funKind == FunctionKind::ECRecover)
+	{
+		// Clears 32 bytes of currently free memory and advances free memory pointer.
+		// Output area will be "start of input area" - 32.
+		// The reason is that a failing ECRecover cannot be detected, it will just return
+		// zero bytes (which we cannot detect).
+		solAssert(0 < retSize && retSize <= 32, "");
+		utils().fetchFreeMemoryPointer();
+		m_context << Instruction::DUP1 << u256(0) << Instruction::MSTORE;
+		m_context << u256(32) << Instruction::ADD;
+		utils().storeFreeMemoryPointer();
+	}
+
 	// Copy function identifier to memory.
 	utils().fetchFreeMemoryPointer();
 	if (!_functionType.isBareCall() || manualFunctionId)
@@ -1453,7 +1466,7 @@ void ExpressionCompiler::appendExternalFunctionCall(
 		utils().storeInMemoryDynamic(IntegerType(8 * CompilerUtils::dataStartOffset), false);
 	}
 	// If the function takes arbitrary parameters, copy dynamic length data in place.
-	// Move argumenst to memory, will not update the free memory pointer (but will update the memory
+	// Move arguments to memory, will not update the free memory pointer (but will update the memory
 	// pointer on the stack).
 	utils().encodeToMemory(
 		argumentTypes,
@@ -1471,12 +1484,24 @@ void ExpressionCompiler::appendExternalFunctionCall(
 	// function identifier [unless bare]
 	// contract address
 
-	// Output data will replace input data.
+	// Output data will replace input data, unless we have ECRecover (then, output
+	// area will be 32 bytes just before input area).
 	// put on stack: <size of output> <memory pos of output> <size of input> <memory pos of input>
 	m_context << u256(retSize);
-	utils().fetchFreeMemoryPointer();
-	m_context << Instruction::DUP1 << Instruction::DUP4 << Instruction::SUB;
-	m_context << Instruction::DUP2;
+	utils().fetchFreeMemoryPointer(); // This is the start of input
+	if (funKind == FunctionKind::ECRecover)
+	{
+		// In this case, output is 32 bytes before input and has already been cleared.
+		m_context << u256(32) << Instruction::DUP2 << Instruction::SUB << Instruction::SWAP1;
+		// Here: <input end> <output size> <outpos> <input pos>
+		m_context << Instruction::DUP1 << Instruction::DUP5 << Instruction::SUB;
+		m_context << Instruction::SWAP1;
+	}
+	else
+	{
+		m_context << Instruction::DUP1 << Instruction::DUP4 << Instruction::SUB;
+		m_context << Instruction::DUP2;
+	}
 
 	// CALL arguments: outSize, outOff, inSize, inOff (already present up to here)
 	// [value,] addr, gas (stack top)
@@ -1539,6 +1564,16 @@ void ExpressionCompiler::appendExternalFunctionCall(
 		utils().loadFromMemoryDynamic(IntegerType(160), false, true, false);
 		utils().convertType(IntegerType(160), FixedBytesType(20));
 	}
+	else if (funKind == FunctionKind::ECRecover)
+	{
+		// Output is 32 bytes before input / free mem pointer.
+		// Failing ecrecover cannot be detected, so we clear output before the call.
+		m_context << u256(32);
+		utils().fetchFreeMemoryPointer();
+		m_context << Instruction::SUB << Instruction::MLOAD;
+		m_context << Instruction::DUP1 << Instruction::ISZERO;
+		m_context.appendConditionalJumpTo(m_context.errorTag());
+	}
 	else if (!_functionType.returnParameterTypes().empty())
 	{
 		utils().fetchFreeMemoryPointer();
diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp
index 1b7c5ea46..338a47da0 100644
--- a/test/libsolidity/SolidityEndToEndTest.cpp
+++ b/test/libsolidity/SolidityEndToEndTest.cpp
@@ -6853,6 +6853,24 @@ BOOST_AUTO_TEST_CASE(create_dynamic_array_with_zero_length)
 	BOOST_CHECK(callContractFunction("f()") == encodeArgs(u256(7)));
 }
 
+BOOST_AUTO_TEST_CASE(failing_ecrecover_invalid_input)
+{
+	// ecrecover should throw for malformed input
+	// (v should be 27 or 28, not 1)
+	// This is quite hard to test because the precompiled does NOT throw, instead it just
+	// does not write to its output area, we have to check that and currently do it
+	// by checking whether ecrecover "returns" zero.
+	char const* sourceCode = R"(
+		contract C {
+			function f() returns (address) {
+				return ecrecover(bytes32(uint(-1)), 1, 2, 3);
+			}
+		}
+	)";
+	compileAndRun(sourceCode, 0, "C");
+	BOOST_CHECK(callContractFunction("f()") == encodeArgs());
+}
+
 BOOST_AUTO_TEST_SUITE_END()
 
 }