Change invalid opcode to revert for input validation.

6 files changed, 52 insertions, 20 deletions

diff --git a/libsolidity/codegen/CompilerContext.cpp b/libsolidity/codegen/CompilerContext.cpp
index 9d0d6d37..1937b529 100644
--- a/libsolidity/codegen/CompilerContext.cpp
+++ b/libsolidity/codegen/CompilerContext.cpp
@@ -244,6 +244,20 @@ CompilerContext& CompilerContext::appendConditionalInvalid()
 	return *this;
 }
 
+CompilerContext& CompilerContext::appendRevert()
+{
+	return *this << u256(0) << u256(0) << Instruction::REVERT;
+}
+
+CompilerContext& CompilerContext::appendConditionalRevert()
+{
+	*this << Instruction::ISZERO;
+	eth::AssemblyItem afterTag = appendConditionalJump();
+	appendRevert();
+	*this << afterTag;
+	return *this;
+}
+
 void CompilerContext::resetVisitedNodes(ASTNode const* _node)
 {
 	stack<ASTNode const*> newStack;
diff --git a/libsolidity/codegen/CompilerContext.h b/libsolidity/codegen/CompilerContext.h
index 030b35a6..1968c1e1 100644
--- a/libsolidity/codegen/CompilerContext.h
+++ b/libsolidity/codegen/CompilerContext.h
@@ -136,11 +136,15 @@ public:
 	/// Appends a JUMP to a new tag and @returns the tag
 	eth::AssemblyItem appendJumpToNew() { return m_asm->appendJump().tag(); }
 	/// Appends a JUMP to a tag already on the stack
-	CompilerContext&  appendJump(eth::AssemblyItem::JumpType _jumpType = eth::AssemblyItem::JumpType::Ordinary);
+	CompilerContext& appendJump(eth::AssemblyItem::JumpType _jumpType = eth::AssemblyItem::JumpType::Ordinary);
 	/// Appends an INVALID instruction
-	CompilerContext&  appendInvalid();
+	CompilerContext& appendInvalid();
 	/// Appends a conditional INVALID instruction
-	CompilerContext&  appendConditionalInvalid();
+	CompilerContext& appendConditionalInvalid();
+	/// Appends a REVERT(0, 0) call
+	CompilerContext& appendRevert();
+	/// Appends a conditional REVERT(0, 0) call
+	CompilerContext& appendConditionalRevert();
 	/// Appends a JUMP to a specific tag
 	CompilerContext& appendJumpTo(eth::AssemblyItem const& _tag) { m_asm->appendJump(_tag); return *this; }
 	/// Appends pushing of a new tag and @returns the new tag.
diff --git a/libsolidity/codegen/CompilerUtils.cpp b/libsolidity/codegen/CompilerUtils.cpp
index bfe72961..7fed1975 100644
--- a/libsolidity/codegen/CompilerUtils.cpp
+++ b/libsolidity/codegen/CompilerUtils.cpp
@@ -392,7 +392,13 @@ void CompilerUtils::pushCombinedFunctionEntryLabel(Declaration const& _function)
 			Instruction::OR;
 }
 
-void CompilerUtils::convertType(Type const& _typeOnStack, Type const& _targetType, bool _cleanupNeeded, bool _chopSignBits)
+void CompilerUtils::convertType(
+	Type const& _typeOnStack,
+	Type const& _targetType,
+	bool _cleanupNeeded,
+	bool _chopSignBits,
+	bool _asPartOfArgumentDecoding
+)
 {
 	// For a type extension, we need to remove all higher-order bits that we might have ignored in
 	// previous operations.
@@ -450,7 +456,10 @@ void CompilerUtils::convertType(Type const& _typeOnStack, Type const& _targetTyp
 			EnumType const& enumType = dynamic_cast<decltype(enumType)>(_typeOnStack);
 			solAssert(enumType.numberOfMembers() > 0, "empty enum should have caused a parser error.");
 			m_context << u256(enumType.numberOfMembers() - 1) << Instruction::DUP2 << Instruction::GT;
-			m_context.appendConditionalInvalid();
+			if (_asPartOfArgumentDecoding)
+				m_context.appendConditionalRevert();
+			else
+				m_context.appendConditionalInvalid();
 			enumOverflowCheckPending = false;
 		}
 		break;
@@ -985,7 +994,7 @@ unsigned CompilerUtils::loadFromMemoryHelper(Type const& _type, bool _fromCallda
 			m_context << shiftFactor << Instruction::MUL;
 	}
 	if (_fromCalldata)
-		convertType(_type, _type, true);
+		convertType(_type, _type, true, false, true);
 
 	return numBytes;
 }
diff --git a/libsolidity/codegen/CompilerUtils.h b/libsolidity/codegen/CompilerUtils.h
index a88951bc..caf2cdc2 100644
--- a/libsolidity/codegen/CompilerUtils.h
+++ b/libsolidity/codegen/CompilerUtils.h
@@ -135,7 +135,15 @@ public:
 	/// If @a _cleanupNeeded, high order bits cleanup is also done if no type conversion would be
 	/// necessary.
 	/// If @a _chopSignBits, the function resets the signed bits out of the width of the signed integer.
-	void convertType(Type const& _typeOnStack, Type const& _targetType, bool _cleanupNeeded = false, bool _chopSignBits = false);
+	/// If @a _asPartOfArgumentDecoding is true, failed conversions are flagged via REVERT,
+	/// otherwise they are flagged with INVALID.
+	void convertType(
+		Type const& _typeOnStack,
+		Type const& _targetType,
+		bool _cleanupNeeded = false,
+		bool _chopSignBits = false,
+		bool _asPartOfArgumentDecoding = false
+	);
 
 	/// Creates a zero-value for the given type and puts it onto the stack. This might allocate
 	/// memory for memory references.
diff --git a/libsolidity/codegen/ContractCompiler.cpp b/libsolidity/codegen/ContractCompiler.cpp
index dc090634..ace82ad4 100644
--- a/libsolidity/codegen/ContractCompiler.cpp
+++ b/libsolidity/codegen/ContractCompiler.cpp
@@ -111,7 +111,7 @@ void ContractCompiler::appendCallValueCheck()
 {
 	// Throw if function is not payable but call contained ether.
 	m_context << Instruction::CALLVALUE;
-	m_context.appendConditionalInvalid();
+	m_context.appendConditionalRevert();
 }
 
 void ContractCompiler::appendInitAndConstructorCode(ContractDefinition const& _contract)
@@ -276,7 +276,7 @@ void ContractCompiler::appendFunctionSelector(ContractDefinition const& _contrac
 		appendReturnValuePacker(FunctionType(*fallback).returnParameterTypes(), _contract.isLibrary());
 	}
 	else
-		m_context.appendInvalid();
+		m_context.appendRevert();
 
 	for (auto const& it: interfaceFunctions)
 	{
@@ -368,7 +368,7 @@ void ContractCompiler::appendCalldataUnpacker(TypePointers const& _typeParameter
 					// copy to memory
 					// move calldata type up again
 					CompilerUtils(m_context).moveIntoStack(calldataType->sizeOnStack());
-					CompilerUtils(m_context).convertType(*calldataType, arrayType);
+					CompilerUtils(m_context).convertType(*calldataType, arrayType, false, false, true);
 					// fetch next pointer again
 					CompilerUtils(m_context).moveToStackTop(arrayType.sizeOnStack());
 				}
@@ -805,8 +805,7 @@ bool ContractCompiler::visit(Throw const& _throw)
 {
 	CompilerContext::LocationSetter locationSetter(m_context, _throw);
 	// Do not send back an error detail.
-	m_context << u256(0) << u256(0);
-	m_context << Instruction::REVERT;
+	m_context.appendRevert();
 	return false;
 }
 
diff --git a/libsolidity/codegen/ExpressionCompiler.cpp b/libsolidity/codegen/ExpressionCompiler.cpp
index 03bba80c..2fc3bf6b 100644
--- a/libsolidity/codegen/ExpressionCompiler.cpp
+++ b/libsolidity/codegen/ExpressionCompiler.cpp
@@ -587,7 +587,7 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall)
 			m_context << Instruction::CREATE;
 			// Check if zero (out of stack or not enough balance).
 			m_context << Instruction::DUP1 << Instruction::ISZERO;
-			m_context.appendConditionalInvalid();
+			m_context.appendConditionalRevert();
 			if (function.valueSet())
 				m_context << swapInstruction(1) << Instruction::POP;
 			break;
@@ -651,7 +651,7 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall)
 			{
 				// Check if zero (out of stack or not enough balance).
 				m_context << Instruction::ISZERO;
-				m_context.appendConditionalInvalid();
+				m_context.appendConditionalRevert();
 			}
 			break;
 		case FunctionType::Kind::Selfdestruct:
@@ -660,9 +660,7 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall)
 			m_context << Instruction::SELFDESTRUCT;
 			break;
 		case FunctionType::Kind::Revert:
-			// memory offset returned - zero length
-			m_context << u256(0) << u256(0);
-			m_context << Instruction::REVERT;
+			m_context.appendRevert();
 			break;
 		case FunctionType::Kind::SHA3:
 		{
@@ -890,7 +888,7 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall)
 				// condition was not met, flag an error
 				m_context << Instruction::INVALID;
 			else
-				m_context << u256(0) << u256(0) << Instruction::REVERT;
+				m_context.appendRevert();
 			// the success branch
 			m_context << success;
 			break;
@@ -1695,7 +1693,7 @@ void ExpressionCompiler::appendExternalFunctionCall(
 	if (funKind == FunctionType::Kind::External || funKind == FunctionType::Kind::CallCode || funKind == FunctionType::Kind::DelegateCall)
 	{
 		m_context << Instruction::DUP1 << Instruction::EXTCODESIZE << Instruction::ISZERO;
-		m_context.appendConditionalInvalid();
+		m_context.appendConditionalRevert();
 		existenceChecked = true;
 	}
 
@@ -1731,7 +1729,7 @@ void ExpressionCompiler::appendExternalFunctionCall(
 	{
 		//Propagate error condition (if CALL pushes 0 on stack).
 		m_context << Instruction::ISZERO;
-		m_context.appendConditionalInvalid();
+		m_context.appendConditionalRevert();
 	}
 
 	utils().popStackSlots(remainsSize);