Merge pull request #5815 from ethereum/strict-abi-decoder

Strict abi decoder (validate incoming data instead of cleaning it)

Changelog.md

@@ -5,6 +5,7 @@ Language Features:
 
 
 Compiler Features:
+ * ABI Decoder: Raise a runtime error on dirty inputs when using the experimental decoder.
  * SMTChecker: Support arithmetic compound assignment operators.
  * Optimizer: Add rule for shifts by constants larger than 255 for Constantinople.
  * Optimizer: Add rule to simplify certain ANDs and SHL combinations

libsolidity/codegen/ABIFunctions.cpp

@@ -255,10 +255,9 @@ string ABIFunctions::EncodingOptions::toFunctionNameSuffix() const
 	return suffix;
 }
 
-
+string ABIFunctions::cleanupFunction(Type const& _type)
-string ABIFunctions::cleanupFunction(Type const& _type, bool _revertOnFailure)
 {
-	string functionName = string("cleanup_") + (_revertOnFailure ? "revert_" : "assert_") + _type.identifier();
+	string functionName = string("cleanup_") + _type.identifier();
 	return createFunction(functionName, [&]() {
 		Whiskers templ(R"(
 			function <functionName>(value) -> cleaned {
@@ -269,7 +268,7 @@ string ABIFunctions::cleanupFunction(Type const& _type, bool _revertOnFailure)
 		switch (_type.category())
 		{
 		case Type::Category::Address:
-			templ("body", "cleaned := " + cleanupFunction(IntegerType(160), _revertOnFailure) + "(value)");
+			templ("body", "cleaned := " + cleanupFunction(IntegerType(160)) + "(value)");
 			break;
 		case Type::Category::Integer:
 		{
@@ -291,6 +290,10 @@ string ABIFunctions::cleanupFunction(Type const& _type, bool _revertOnFailure)
 		case Type::Category::FixedPoint:
 			solUnimplemented("Fixed point types not implemented.");
 			break;
+		case Type::Category::Function:
+			solAssert(dynamic_cast<FunctionType const&>(_type).kind() == FunctionType::Kind::External, "");
+			templ("body", "cleaned := " + cleanupFunction(FixedBytesType(24)) + "(value)");
+			break;
 		case Type::Category::Array:
 		case Type::Category::Struct:
 		case Type::Category::Mapping:
@@ -319,20 +322,13 @@ string ABIFunctions::cleanupFunction(Type const& _type, bool _revertOnFailure)
 				StateMutability::Payable :
 				StateMutability::NonPayable
 			);
-			templ("body", "cleaned := " + cleanupFunction(addressType, _revertOnFailure) + "(value)");
+			templ("body", "cleaned := " + cleanupFunction(addressType) + "(value)");
 			break;
 		}
 		case Type::Category::Enum:
 		{
-			size_t members = dynamic_cast<EnumType const&>(_type).numberOfMembers();
+			// Out of range enums cannot be truncated unambigiously and therefore it should be an error.
-			solAssert(members > 0, "empty enum should have caused a parser error.");
+			templ("body", "cleaned := value " + validatorFunction(_type) + "(value)");
-			Whiskers w("if iszero(lt(value, <members>)) { <failure> } cleaned := value");
-			w("members", to_string(members));
-			if (_revertOnFailure)
-				w("failure", "revert(0, 0)");
-			else
-				w("failure", "invalid()");
-			templ("body", w.render());
 			break;
 		}
 		case Type::Category::InaccessibleDynamic:
@@ -346,6 +342,56 @@ string ABIFunctions::cleanupFunction(Type const& _type, bool _revertOnFailure)
 	});
 }
 
+string ABIFunctions::validatorFunction(Type const& _type, bool _revertOnFailure)
+{
+	string functionName = string("validator_") + (_revertOnFailure ? "revert_" : "assert_") + _type.identifier();
+	return createFunction(functionName, [&]() {
+		Whiskers templ(R"(
+			function <functionName>(value) {
+				if iszero(<condition>) { <failure> }
+			}
+		)");
+		templ("functionName", functionName);
+		if (_revertOnFailure)
+			templ("failure", "revert(0, 0)");
+		else
+			templ("failure", "invalid()");
+
+		switch (_type.category())
+		{
+		case Type::Category::Address:
+		case Type::Category::Integer:
+		case Type::Category::RationalNumber:
+		case Type::Category::Bool:
+		case Type::Category::FixedPoint:
+		case Type::Category::Function:
+		case Type::Category::Array:
+		case Type::Category::Struct:
+		case Type::Category::Mapping:
+		case Type::Category::FixedBytes:
+		case Type::Category::Contract:
+		{
+			templ("condition", "eq(value, " + cleanupFunction(_type) + "(value))");
+			break;
+		}
+		case Type::Category::Enum:
+		{
+			size_t members = dynamic_cast<EnumType const&>(_type).numberOfMembers();
+			solAssert(members > 0, "empty enum should have caused a parser error.");
+			templ("condition", "lt(value, " + to_string(members) + ")");
+			break;
+		}
+		case Type::Category::InaccessibleDynamic:
+			templ("condition", "1");
+			break;
+		default:
+			solAssert(false, "Validation of type " + _type.identifier() + " requested.");
+		}
+
+		return templ.render();
+	});
+}
+
 string ABIFunctions::cleanupFromStorageFunction(Type const& _type, bool _splitFunctionTypes)
 {
 	solAssert(_type.isValueType(), "");
@@ -544,21 +590,6 @@ string ABIFunctions::conversionFunction(Type const& _from, Type const& _to)
 	});
 }
 
-string ABIFunctions::cleanupCombinedExternalFunctionIdFunction()
-{
-	string functionName = "cleanup_combined_external_function_id";
-	return createFunction(functionName, [&]() {
-		return Whiskers(R"(
-			function <functionName>(addr_and_selector) -> cleaned {
-				cleaned := <clean>(addr_and_selector)
-			}
-		)")
-		("functionName", functionName)
-		("clean", cleanupFunction(FixedBytesType(24)))
-		.render();
-	});
-}
-
 string ABIFunctions::abiEncodingFunction(
 	Type const& _from,
 	Type const& _to,
@@ -1248,7 +1279,7 @@ string ABIFunctions::abiEncodingFunctionFunctionType(
 				}
 			)")
 			("functionName", functionName)
-			("cleanExtFun", cleanupCombinedExternalFunctionIdFunction())
+			("cleanExtFun", cleanupFunction(_to))
 			.render();
 		});
 }
@@ -1306,14 +1337,15 @@ string ABIFunctions::abiDecodingFunctionValueType(Type const& _type, bool _fromM
 	return createFunction(functionName, [&]() {
 		Whiskers templ(R"(
 			function <functionName>(offset, end) -> value {
-				value := <cleanup>(<load>(offset))
+				value := <load>(offset)
+				<validator>(value)
 			}
 		)");
 		templ("functionName", functionName);
 		templ("load", _fromMemory ? "mload" : "calldataload");
-		// Cleanup itself should use the type and not decodingType, because e.g.
+		// Validation should use the type and not decodingType, because e.g.
 		// the decoding type of an enum is a plain int.
-		templ("cleanup", cleanupFunction(_type, true));
+		templ("validator", validatorFunction(_type, true));
 		return templ.render();
 	});
 
@@ -1560,11 +1592,11 @@ string ABIFunctions::abiDecodingFunctionFunctionType(FunctionType const& _type,
 		{
 			return Whiskers(R"(
 				function <functionName>(offset, end) -> addr, function_selector {
-					addr, function_selector := <splitExtFun>(<load>(offset))
+					addr, function_selector := <splitExtFun>(<decodeFun>(offset, end))
 				}
 			)")
 			("functionName", functionName)
-			("load", _fromMemory ? "mload" : "calldataload")
+			("decodeFun", abiDecodingFunctionFunctionType(_type, _fromMemory, false))
 			("splitExtFun", m_utils.splitExternalFunctionIdFunction())
 			.render();
 		}
@@ -1572,12 +1604,13 @@ string ABIFunctions::abiDecodingFunctionFunctionType(FunctionType const& _type,
 		{
 			return Whiskers(R"(
 				function <functionName>(offset, end) -> fun {
-					fun := <cleanExtFun>(<load>(offset))
+					fun := <load>(offset)
+					<validateExtFun>(fun)
 				}
 			)")
 			("functionName", functionName)
 			("load", _fromMemory ? "mload" : "calldataload")
-			("cleanExtFun", cleanupCombinedExternalFunctionIdFunction())
+			("validateExtFun", validatorFunction(_type, true))
 			.render();
 		}
 	});

libsolidity/codegen/ABIFunctions.h

@@ -130,9 +130,17 @@ private:
 
 	/// @returns the name of the cleanup function for the given type and
 	/// adds its implementation to the requested functions.
+	/// The cleanup function defers to the validator function with "assert"
+	/// if there is no reasonable way to clean a value.
+	std::string cleanupFunction(Type const& _type);
+
+	/// @returns the name of the validator function for the given type and
+	/// adds its implementation to the requested functions.
 	/// @param _revertOnFailure if true, causes revert on invalid data,
 	/// otherwise an assertion failure.
-	std::string cleanupFunction(Type const& _type, bool _revertOnFailure = false);
+	///
+	/// This is used for data decoded from external sources.
+	std::string validatorFunction(Type const& _type, bool _revertOnFailure = false);
 
 	/// Performs cleanup after reading from a potentially compressed storage slot.
 	/// The function does not perform any validation, it just masks or sign-extends
@@ -146,10 +154,10 @@ private:
 	/// @returns the name of the function that converts a value of type @a _from
 	/// to a value of type @a _to. The resulting vale is guaranteed to be in range
 	/// (i.e. "clean"). Asserts on failure.
+	///
+	/// This is used for data being encoded or general type conversions in the code.
 	std::string conversionFunction(Type const& _from, Type const& _to);
 
-	std::string cleanupCombinedExternalFunctionIdFunction();
-
 	/// @returns the name of the ABI encoding function with the given type
 	/// and queues the generation of the function to the requested functions.
 	/// @param _fromStack if false, the input value was just loaded from storage

libsolidity/codegen/ExpressionCompiler.cpp

@@ -1406,6 +1406,7 @@ bool ExpressionCompiler::visit(MemberAccess const& _memberAccess)
 	case Type::Category::Struct:
 	{
 		StructType const& type = dynamic_cast<StructType const&>(*_memberAccess.expression().annotation().type);
+		TypePointer const& memberType = _memberAccess.annotation().type;
 		switch (type.location())
 		{
 		case DataLocation::Storage:
@@ -1418,7 +1419,7 @@ bool ExpressionCompiler::visit(MemberAccess const& _memberAccess)
 		case DataLocation::Memory:
 		{
 			m_context << type.memoryOffsetOfMember(member) << Instruction::ADD;
-			setLValue<MemoryItem>(_memberAccess, *_memberAccess.annotation().type);
+			setLValue<MemoryItem>(_memberAccess, *memberType);
 			break;
 		}
 		case DataLocation::CallData:
@@ -1427,21 +1428,28 @@ bool ExpressionCompiler::visit(MemberAccess const& _memberAccess)
 			{
 				m_context << Instruction::DUP1;
 				m_context << type.calldataOffsetOfMember(member) << Instruction::ADD;
-				CompilerUtils(m_context).accessCalldataTail(*_memberAccess.annotation().type);
+				CompilerUtils(m_context).accessCalldataTail(*memberType);
 			}
 			else
 			{
 				m_context << type.calldataOffsetOfMember(member) << Instruction::ADD;
 				// For non-value types the calldata offset is returned directly.
-				if (_memberAccess.annotation().type->isValueType())
+				if (memberType->isValueType())
 				{
-					solAssert(_memberAccess.annotation().type->calldataEncodedSize() > 0, "");
+					solAssert(memberType->calldataEncodedSize() > 0, "");
-					CompilerUtils(m_context).loadFromMemoryDynamic(*_memberAccess.annotation().type, true, true, false);
+					solAssert(memberType->storageBytes() <= 32, "");
+					if (memberType->storageBytes() < 32 && m_context.experimentalFeatureActive(ExperimentalFeature::ABIEncoderV2))
+					{
+						m_context << u256(32);
+						CompilerUtils(m_context).abiDecodeV2({memberType}, false);
+					}
+					else
+						CompilerUtils(m_context).loadFromMemoryDynamic(*memberType, true, true, false);
 				}
 				else
 					solAssert(
-						_memberAccess.annotation().type->category() == Type::Category::Array ||
+						memberType->category() == Type::Category::Array ||
-						_memberAccess.annotation().type->category() == Type::Category::Struct,
+						memberType->category() == Type::Category::Struct,
 						""
 					);
 			}
@@ -1588,12 +1596,25 @@ bool ExpressionCompiler::visit(IndexAccess const& _indexAccess)
 			{
 				ArrayUtils(m_context).accessIndex(arrayType, true);
 				if (arrayType.baseType()->isValueType())
-					CompilerUtils(m_context).loadFromMemoryDynamic(
+				{
-						*arrayType.baseType(),
+					solAssert(arrayType.baseType()->storageBytes() <= 32, "");
-						true,
+					if (
-						!arrayType.isByteArray(),
+						!arrayType.isByteArray() &&
-						false
+						arrayType.baseType()->storageBytes() < 32 &&
-					);
+						m_context.experimentalFeatureActive(ExperimentalFeature::ABIEncoderV2)
+					)
+					{
+						m_context << u256(32);
+						CompilerUtils(m_context).abiDecodeV2({arrayType.baseType()}, false);
+					}
+					else
+						CompilerUtils(m_context).loadFromMemoryDynamic(
+							*arrayType.baseType(),
+							true,
+							!arrayType.isByteArray(),
+							false
+						);
+				}
 				else
 					solAssert(
 						arrayType.baseType()->category() == Type::Category::Struct ||

test/cmdlineTests/gas_test_abiv2/output

@@ -2,11 +2,11 @@
 ======= gas_test_abiv2/input.sol:C =======
 Gas estimation:
 construction:
-   1140 + 1096600 = 1097740
+   1160 + 1115800 = 1116960
 external:
    a():	530
-   b(uint256):	1118
+   b(uint256):	infinite
-   f1(uint256):	586
+   f1(uint256):	infinite
    f2(uint256[],string[],uint16,address):	infinite
    f3(uint16[],string[],uint16,address):	infinite
    f4(uint32[],string[12],bytes[2][],address):	infinite

test/cmdlineTests/gas_test_abiv2_optimize_yul/output

@@ -2,7 +2,7 @@
 ======= gas_test_abiv2_optimize_yul/input.sol:C =======
 Gas estimation:
 construction:
-   651 + 617200 = 617851
+   651 + 616600 = 617251
 external:
    a():	429
    b(uint256):	884

test/contracts/Wallet.cpp

@@ -464,7 +464,8 @@ BOOST_AUTO_TEST_CASE(creation)
 {
 	deployWallet(200);
 	BOOST_REQUIRE(callContractFunction("isOwner(address)", h256(m_sender, h256::AlignRight)) == encodeArgs(true));
-	BOOST_REQUIRE(callContractFunction("isOwner(address)", ~h256(m_sender, h256::AlignRight)) == encodeArgs(false));
+	bool v2 = dev::test::Options::get().useABIEncoderV2;
+	BOOST_REQUIRE(callContractFunction("isOwner(address)", ~h256(m_sender, h256::AlignRight)) == (v2 ? encodeArgs() : encodeArgs(false)));
 }
 
 BOOST_AUTO_TEST_CASE(add_owners)

test/libsolidity/ABIDecoderTests.cpp

@@ -108,6 +108,7 @@ BOOST_AUTO_TEST_CASE(cleanup)
 			}
 		}
 	)";
+	bool newDecoder = dev::test::Options::get().useABIEncoderV2;
 	BOTH_ENCODERS(
 		compileAndRun(sourceCode);
 		ABI_CHECK(
@@ -117,10 +118,46 @@ BOOST_AUTO_TEST_CASE(cleanup)
 		ABI_CHECK(
 			callContractFunction(
 				"f(uint16,int16,address,bytes3,bool)",
-				u256(0xffffff), u256(0x1ffff), u256(-1), string("abcd"), u256(4)
+				u256(0xffffff), u256(0x1ffff), u256(-1), string("abcd"), u256(1)
 			),
-			encodeArgs(u256(0xffff), u256(-1), (u256(1) << 160) - 1, string("abc"), true)
+			newDecoder ? bytes{} : encodeArgs(u256(0xffff), u256(-1), (u256(1) << 160) - 1, string("abc"), true)
 		);
+		ABI_CHECK(
+			callContractFunction(
+				"f(uint16,int16,address,bytes3,bool)",
+				u256(0xffffff), u256(0), u256(0), string("bcd"), u256(1)
+			),
+			newDecoder ? bytes{} : encodeArgs(u256(0xffff), u256(0), 0, string("bcd"), true)
+		);
+		ABI_CHECK(
+			callContractFunction(
+				"f(uint16,int16,address,bytes3,bool)",
+				u256(0), u256(0x1ffff), u256(0), string("ab"), u256(1)
+			),
+			newDecoder ? bytes{} : encodeArgs(u256(0), u256(-1), 0, string("ab"), true)
+		);
+		ABI_CHECK(
+			callContractFunction(
+				"f(uint16,int16,address,bytes3,bool)",
+				u256(0), u256(0), u256(-1), string("ad"), u256(1)
+			),
+			newDecoder ? bytes{} : encodeArgs(u256(0), u256(0), (u256(1) << 160) - 1, string("ad"), true)
+		);
+		ABI_CHECK(
+			callContractFunction(
+				"f(uint16,int16,address,bytes3,bool)",
+				u256(0), u256(0), u256(0), string("abcd"), u256(1)
+			),
+			newDecoder ? bytes{} : encodeArgs(u256(0), u256(0), 0, string("abc"), true)
+		);
+		ABI_CHECK(
+			callContractFunction(
+				"f(uint16,int16,address,bytes3,bool)",
+				u256(0), u256(0), u256(0), string("abc"), u256(2)
+			),
+			newDecoder ? bytes{} : encodeArgs(u256(0), u256(0), 0, string("abc"), true)
+		);
+		newDecoder = true;
 	)
 }
 
@@ -506,7 +543,7 @@ BOOST_AUTO_TEST_CASE(short_input_bytes)
 	)
 }
 
-BOOST_AUTO_TEST_CASE(cleanup_int_inside_arrays)
+BOOST_AUTO_TEST_CASE(validation_int_inside_arrays)
 {
 	string sourceCode = R"(
 		contract C {
@@ -521,15 +558,69 @@ BOOST_AUTO_TEST_CASE(cleanup_int_inside_arrays)
 		ABI_CHECK(callContractFunction("f(uint16[])", 0x20, 1, 7), encodeArgs(7));
 		ABI_CHECK(callContractFunction("g(int16[])", 0x20, 1, 7), encodeArgs(7));
 		ABI_CHECK(callContractFunction("f(uint16[])", 0x20, 1, u256("0xffff")), encodeArgs(u256("0xffff")));
-		ABI_CHECK(callContractFunction("g(int16[])", 0x20, 1, u256("0xffff")), encodeArgs(u256(-1)));
+		ABI_CHECK(callContractFunction("g(int16[])", 0x20, 1, u256("0xffff")), encodeArgs());
-		ABI_CHECK(callContractFunction("f(uint16[])", 0x20, 1, u256("0x1ffff")), encodeArgs(u256("0xffff")));
+		ABI_CHECK(callContractFunction("f(uint16[])", 0x20, 1, u256("0x1ffff")), encodeArgs());
-		ABI_CHECK(callContractFunction("g(int16[])", 0x20, 1, u256("0x10fff")), encodeArgs(u256("0x0fff")));
+		ABI_CHECK(callContractFunction("g(int16[])", 0x20, 1, u256("0x10fff")), encodeArgs());
 		ABI_CHECK(callContractFunction("h(uint8[])", 0x20, 1, 0), encodeArgs(u256(0)));
 		ABI_CHECK(callContractFunction("h(uint8[])", 0x20, 1, 1), encodeArgs(u256(1)));
 		ABI_CHECK(callContractFunction("h(uint8[])", 0x20, 1, 2), encodeArgs());
 	)
 }
 
+BOOST_AUTO_TEST_CASE(validation_function_type)
+{
+	string sourceCode = R"(
+		contract C {
+			function f(function () external) public pure returns (uint r) { r = 1; }
+			function g(function () external[] memory) public pure returns (uint r) { r = 2; }
+			function h(function () external[] calldata) external pure returns (uint r) { r = 3; }
+			function i(function () external[] calldata a) external pure returns (uint r) { a[0]; r = 4; }
+		}
+	)";
+	bool newDecoder = dev::test::Options::get().useABIEncoderV2;
+	string validFun{"01234567890123456789abcd"};
+	string invalidFun{"01234567890123456789abcdX"};
+	BOTH_ENCODERS(
+		compileAndRun(sourceCode);
+		ABI_CHECK(callContractFunction("f(function)", validFun), encodeArgs(1));
+		ABI_CHECK(callContractFunction("f(function)", invalidFun), newDecoder ? bytes{} : encodeArgs(1));
+		ABI_CHECK(callContractFunction("g(function[])", 0x20, 1, validFun), encodeArgs(2));
+		ABI_CHECK(callContractFunction("g(function[])", 0x20, 1, invalidFun), newDecoder ? bytes{} : encodeArgs(2));
+		ABI_CHECK(callContractFunction("h(function[])", 0x20, 1, validFun), encodeArgs(3));
+		// No failure because the data is not accessed.
+		ABI_CHECK(callContractFunction("h(function[])", 0x20, 1, invalidFun), encodeArgs(3));
+		ABI_CHECK(callContractFunction("i(function[])", 0x20, 1, validFun), encodeArgs(4));
+		ABI_CHECK(callContractFunction("i(function[])", 0x20, 1, invalidFun), newDecoder ? bytes{} : encodeArgs(4));
+		newDecoder = true;
+	)
+}
+
+BOOST_AUTO_TEST_CASE(validation_function_type_inside_struct)
+{
+	string sourceCode = R"(
+		contract C {
+			struct S { function () external x; }
+			function f(S memory) public pure returns (uint r) { r = 1; }
+			function g(S calldata) external pure returns (uint r) { r = 2; }
+			function h(S calldata s) external pure returns (uint r) { s.x; r = 3; }
+		}
+	)";
+	string validFun{"01234567890123456789abcd"};
+	string invalidFun{"01234567890123456789abcdX"};
+	NEW_ENCODER(
+		compileAndRun(sourceCode);
+		ABI_CHECK(callContractFunction("f((function))", validFun), encodeArgs(1));
+		// Error because we copy to memory
+		ABI_CHECK(callContractFunction("f((function))", invalidFun), encodeArgs());
+		ABI_CHECK(callContractFunction("g((function))", validFun), encodeArgs(2));
+		// No error because x is not accessed.
+		ABI_CHECK(callContractFunction("g((function))", invalidFun), encodeArgs(2));
+		ABI_CHECK(callContractFunction("h((function))", validFun), encodeArgs(3));
+		// Error on access.
+		ABI_CHECK(callContractFunction("h((function))", invalidFun), encodeArgs());
+	)
+}
+
 BOOST_AUTO_TEST_CASE(storage_ptr)
 {
 	string sourceCode = R"(
@@ -583,7 +674,7 @@ BOOST_AUTO_TEST_CASE(struct_simple)
 	)
 }
 
-BOOST_AUTO_TEST_CASE(struct_cleanup)
+BOOST_AUTO_TEST_CASE(struct_validation)
 {
 	string sourceCode = R"(
 		contract C {
@@ -597,11 +688,24 @@ BOOST_AUTO_TEST_CASE(struct_cleanup)
 			}
 		}
 	)";
+	u256 largeNeg("0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff01");
 	NEW_ENCODER(
 		compileAndRun(sourceCode, 0, "C");
 		ABI_CHECK(
-			callContractFunction("f((int16,uint8,bytes2))", 0xff010, 0xff0002, "abcd"),
+			callContractFunction("f((int16,uint8,bytes2))", largeNeg, 0xff, "ab"),
-			encodeArgs(u256("0xfffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff010"), 2, "ab")
+			encodeArgs(largeNeg, 0xff, "ab")
+		);
+		ABI_CHECK(
+			callContractFunction("f((int16,uint8,bytes2))", 0xff010, 0xff, "ab"),
+			encodeArgs()
+		);
+		ABI_CHECK(
+			callContractFunction("f((int16,uint8,bytes2))", largeNeg, 0xff0002, "ab"),
+			encodeArgs()
+		);
+		ABI_CHECK(
+			callContractFunction("f((int16,uint8,bytes2))", largeNeg, 0xff, "abcd"),
+			encodeArgs()
 		);
 	)
 }
@@ -759,7 +863,6 @@ BOOST_AUTO_TEST_CASE(complex_struct)
 	)
 }
 
-
 BOOST_AUTO_TEST_CASE(return_dynamic_types_cross_call_simple)
 {
 	if (m_evmVersion == langutil::EVMVersion::homestead())
@@ -844,6 +947,24 @@ BOOST_AUTO_TEST_CASE(return_dynamic_types_cross_call_out_of_range)
 	)
 }
 
+BOOST_AUTO_TEST_CASE(out_of_bounds_bool_value)
+{
+	string sourceCode = R"(
+		contract C {
+			function f(bool b) public pure returns (bool) { return b; }
+		}
+	)";
+	bool newDecoder = dev::test::Options::get().useABIEncoderV2;
+	BOTH_ENCODERS(
+		compileAndRun(sourceCode);
+		ABI_CHECK(callContractFunction("f(bool)", true), encodeArgs(true));
+		ABI_CHECK(callContractFunction("f(bool)", false), encodeArgs(false));
+		ABI_CHECK(callContractFunctionNoEncoding("f(bool)", bytes(32, 0)), encodeArgs(0));
+		ABI_CHECK(callContractFunctionNoEncoding("f(bool)", bytes(32, 0xff)), newDecoder ? encodeArgs() : encodeArgs(1));
+		newDecoder = true;
+	)
+}
+
 BOOST_AUTO_TEST_SUITE_END()
 
 }

test/libsolidity/SolidityEndToEndTest.cpp

@@ -6773,16 +6773,17 @@ BOOST_AUTO_TEST_CASE(bool_conversion)
 		}
 	)";
 	compileAndRun(sourceCode, 0, "C");
+	bool v2 = dev::test::Options::get().useABIEncoderV2;
 	ABI_CHECK(callContractFunction("f(bool)", 0), encodeArgs(0));
 	ABI_CHECK(callContractFunction("f(bool)", 1), encodeArgs(1));
-	ABI_CHECK(callContractFunction("f(bool)", 2), encodeArgs(1));
+	ABI_CHECK(callContractFunction("f(bool)", 2), v2 ? encodeArgs() : encodeArgs(1));
-	ABI_CHECK(callContractFunction("f(bool)", 3), encodeArgs(1));
+	ABI_CHECK(callContractFunction("f(bool)", 3), v2 ? encodeArgs() : encodeArgs(1));
-	ABI_CHECK(callContractFunction("f(bool)", 255), encodeArgs(1));
+	ABI_CHECK(callContractFunction("f(bool)", 255), v2 ? encodeArgs() : encodeArgs(1));
 	ABI_CHECK(callContractFunction("g(bool)", 0), encodeArgs(0));
 	ABI_CHECK(callContractFunction("g(bool)", 1), encodeArgs(1));
-	ABI_CHECK(callContractFunction("g(bool)", 2), encodeArgs(1));
+	ABI_CHECK(callContractFunction("g(bool)", 2), v2 ? encodeArgs() : encodeArgs(1));
-	ABI_CHECK(callContractFunction("g(bool)", 3), encodeArgs(1));
+	ABI_CHECK(callContractFunction("g(bool)", 3), v2 ? encodeArgs() : encodeArgs(1));
-	ABI_CHECK(callContractFunction("g(bool)", 255), encodeArgs(1));
+	ABI_CHECK(callContractFunction("g(bool)", 255), v2 ? encodeArgs() : encodeArgs(1));
 }
 
 BOOST_AUTO_TEST_CASE(packed_storage_signed)
@@ -8244,8 +8245,8 @@ BOOST_AUTO_TEST_CASE(calldata_struct_cleaning)
 
 	// double check that the valid case goes through
 	ABI_CHECK(callContractFunction("f((uint8,bytes1))", u256(0x12), bytes{0x34} + bytes(31,0)), encodeArgs(0x12, bytes{0x34} + bytes(31,0)));
-	ABI_CHECK(callContractFunction("f((uint8,bytes1))", u256(0x1234), bytes{0x56, 0x78} + bytes(30,0)), encodeArgs(0x34, bytes{0x56} + bytes(31,0)));
+	ABI_CHECK(callContractFunction("f((uint8,bytes1))", u256(0x1234), bytes{0x56, 0x78} + bytes(30,0)), encodeArgs());
-	ABI_CHECK(callContractFunction("f((uint8,bytes1))", u256(-1), u256(-1)), encodeArgs(0xFF, bytes{0xFF} + bytes(31,0)));
+	ABI_CHECK(callContractFunction("f((uint8,bytes1))", u256(-1), u256(-1)), encodeArgs());
 }
 
 BOOST_AUTO_TEST_CASE(calldata_struct_function_type)
@@ -11031,7 +11032,8 @@ BOOST_AUTO_TEST_CASE(cleanup_bytes_types)
 	)";
 	compileAndRun(sourceCode, 0, "C");
 	// We input longer data on purpose.
-	ABI_CHECK(callContractFunction("f(bytes2,uint16)", string("abc"), u256(0x040102)), encodeArgs(0));
+	bool v2 = dev::test::Options::get().useABIEncoderV2;
+	ABI_CHECK(callContractFunction("f(bytes2,uint16)", string("abc"), u256(0x040102)), v2 ? encodeArgs() : encodeArgs(0));
 }
 
 BOOST_AUTO_TEST_CASE(cleanup_bytes_types_shortening)
@@ -11068,9 +11070,11 @@ BOOST_AUTO_TEST_CASE(cleanup_address_types)
 		}
 	)";
 	compileAndRun(sourceCode, 0, "C");
+
+	bool v2 = dev::test::Options::get().useABIEncoderV2;
 	// We input longer data on purpose.
-	ABI_CHECK(callContractFunction("f(address)", u256("0xFFFF1234567890123456789012345678901234567890")), encodeArgs(0));
+	ABI_CHECK(callContractFunction("f(address)", u256("0xFFFF1234567890123456789012345678901234567890")), v2 ? encodeArgs() : encodeArgs(0));
-	ABI_CHECK(callContractFunction("g(address)", u256("0xFFFF1234567890123456789012345678901234567890")), encodeArgs(0));
+	ABI_CHECK(callContractFunction("g(address)", u256("0xFFFF1234567890123456789012345678901234567890")), v2 ? encodeArgs() : encodeArgs(0));
 }
 
 BOOST_AUTO_TEST_CASE(cleanup_address_types_shortening)
@@ -12325,8 +12329,9 @@ BOOST_AUTO_TEST_CASE(shift_right_garbled)
 		}
 	)";
 	compileAndRun(sourceCode, 0, "C");
+	bool v2 = dev::test::Options::get().useABIEncoderV2;
 	ABI_CHECK(callContractFunction("f(uint8,uint8)", u256(0x0), u256(4)), encodeArgs(u256(0xf)));
-	ABI_CHECK(callContractFunction("f(uint8,uint8)", u256(0x0), u256(0x1004)), encodeArgs(u256(0xf)));
+	ABI_CHECK(callContractFunction("f(uint8,uint8)", u256(0x0), u256(0x1004)), v2 ? encodeArgs() : encodeArgs(u256(0xf)));
 }
 
 BOOST_AUTO_TEST_CASE(shift_right_garbled_signed)
@@ -12350,16 +12355,17 @@ BOOST_AUTO_TEST_CASE(shift_right_garbled_signed)
 			}
 		)";
 	compileAndRun(sourceCode, 0, "C");
+	bool v2 = dev::test::Options::get().useABIEncoderV2;
 	ABI_CHECK(callContractFunction("f(int8,uint8)", u256(0x0), u256(3)), encodeArgs(u256(-2)));
 	ABI_CHECK(callContractFunction("f(int8,uint8)", u256(0x0), u256(4)), encodeArgs(u256(-1)));
 	ABI_CHECK(callContractFunction("f(int8,uint8)", u256(0x0), u256(0xFF)), encodeArgs(u256(-1)));
-	ABI_CHECK(callContractFunction("f(int8,uint8)", u256(0x0), u256(0x1003)), encodeArgs(u256(-2)));
+	ABI_CHECK(callContractFunction("f(int8,uint8)", u256(0x0), u256(0x1003)), v2 ? encodeArgs() : encodeArgs(u256(-2)));
-	ABI_CHECK(callContractFunction("f(int8,uint8)", u256(0x0), u256(0x1004)), encodeArgs(u256(-1)));
+	ABI_CHECK(callContractFunction("f(int8,uint8)", u256(0x0), u256(0x1004)), v2 ? encodeArgs() : encodeArgs(u256(-1)));
 	ABI_CHECK(callContractFunction("g(int8,uint8)", u256(0x0), u256(3)), encodeArgs(u256(-2)));
 	ABI_CHECK(callContractFunction("g(int8,uint8)", u256(0x0), u256(4)), encodeArgs(u256(-1)));
 	ABI_CHECK(callContractFunction("g(int8,uint8)", u256(0x0), u256(0xFF)), encodeArgs(u256(-1)));
-	ABI_CHECK(callContractFunction("g(int8,uint8)", u256(0x0), u256(0x1003)), encodeArgs(u256(-2)));
+	ABI_CHECK(callContractFunction("g(int8,uint8)", u256(0x0), u256(0x1003)), v2 ? encodeArgs() : encodeArgs(u256(-2)));
-	ABI_CHECK(callContractFunction("g(int8,uint8)", u256(0x0), u256(0x1004)), encodeArgs(u256(-1)));
+	ABI_CHECK(callContractFunction("g(int8,uint8)", u256(0x0), u256(0x1004)), v2 ? encodeArgs() : encodeArgs(u256(-1)));
 }
 
 BOOST_AUTO_TEST_CASE(shift_right_uint32)
@@ -12541,11 +12547,12 @@ BOOST_AUTO_TEST_CASE(shift_right_negative_lvalue_signextend_int8)
 			}
 		)";
 	compileAndRun(sourceCode, 0, "C");
-	ABI_CHECK(callContractFunction("f(int8,int8)", u256(0x99u), u256(0)), encodeArgs(u256(-103)));
+	bool v2 = dev::test::Options::get().useABIEncoderV2;
-	ABI_CHECK(callContractFunction("f(int8,int8)", u256(0x99u), u256(1)), encodeArgs(u256(-52)));
+	ABI_CHECK(callContractFunction("f(int8,int8)", u256(0x99u), u256(0)), v2 ? encodeArgs() : encodeArgs(u256(-103)));
-	ABI_CHECK(callContractFunction("f(int8,int8)", u256(0x99u), u256(2)), encodeArgs(u256(-26)));
+	ABI_CHECK(callContractFunction("f(int8,int8)", u256(0x99u), u256(1)), v2 ? encodeArgs() : encodeArgs(u256(-52)));
-	ABI_CHECK(callContractFunction("f(int8,int8)", u256(0x99u), u256(4)), encodeArgs(u256(-7)));
+	ABI_CHECK(callContractFunction("f(int8,int8)", u256(0x99u), u256(2)), v2 ? encodeArgs() : encodeArgs(u256(-26)));
-	ABI_CHECK(callContractFunction("f(int8,int8)", u256(0x99u), u256(8)), encodeArgs(u256(-1)));
+	ABI_CHECK(callContractFunction("f(int8,int8)", u256(0x99u), u256(4)), v2 ? encodeArgs() : encodeArgs(u256(-7)));
+	ABI_CHECK(callContractFunction("f(int8,int8)", u256(0x99u), u256(8)), v2 ? encodeArgs() : encodeArgs(u256(-1)));
 }
 
 BOOST_AUTO_TEST_CASE(shift_right_negative_lvalue_signextend_int16)
@@ -12558,11 +12565,12 @@ BOOST_AUTO_TEST_CASE(shift_right_negative_lvalue_signextend_int16)
 			}
 		)";
 	compileAndRun(sourceCode, 0, "C");
-	ABI_CHECK(callContractFunction("f(int16,int16)", u256(0xFF99u), u256(0)), encodeArgs(u256(-103)));
+	bool v2 = dev::test::Options::get().useABIEncoderV2;
-	ABI_CHECK(callContractFunction("f(int16,int16)", u256(0xFF99u), u256(1)), encodeArgs(u256(-52)));
+	ABI_CHECK(callContractFunction("f(int16,int16)", u256(0xFF99u), u256(0)), v2 ? encodeArgs() : encodeArgs(u256(-103)));
-	ABI_CHECK(callContractFunction("f(int16,int16)", u256(0xFF99u), u256(2)), encodeArgs(u256(-26)));
+	ABI_CHECK(callContractFunction("f(int16,int16)", u256(0xFF99u), u256(1)), v2 ? encodeArgs() : encodeArgs(u256(-52)));
-	ABI_CHECK(callContractFunction("f(int16,int16)", u256(0xFF99u), u256(4)), encodeArgs(u256(-7)));
+	ABI_CHECK(callContractFunction("f(int16,int16)", u256(0xFF99u), u256(2)), v2 ? encodeArgs() : encodeArgs(u256(-26)));
-	ABI_CHECK(callContractFunction("f(int16,int16)", u256(0xFF99u), u256(8)), encodeArgs(u256(-1)));
+	ABI_CHECK(callContractFunction("f(int16,int16)", u256(0xFF99u), u256(4)), v2 ? encodeArgs() : encodeArgs(u256(-7)));
+	ABI_CHECK(callContractFunction("f(int16,int16)", u256(0xFF99u), u256(8)), v2 ? encodeArgs() : encodeArgs(u256(-1)));
 }
 
 BOOST_AUTO_TEST_CASE(shift_right_negative_lvalue_signextend_int32)
@@ -12575,11 +12583,12 @@ BOOST_AUTO_TEST_CASE(shift_right_negative_lvalue_signextend_int32)
 			}
 		)";
 	compileAndRun(sourceCode, 0, "C");
-	ABI_CHECK(callContractFunction("f(int32,int32)", u256(0xFFFFFF99u), u256(0)), encodeArgs(u256(-103)));
+	bool v2 = dev::test::Options::get().useABIEncoderV2;
-	ABI_CHECK(callContractFunction("f(int32,int32)", u256(0xFFFFFF99u), u256(1)), encodeArgs(u256(-52)));
+	ABI_CHECK(callContractFunction("f(int32,int32)", u256(0xFFFFFF99u), u256(0)), v2 ? encodeArgs() : encodeArgs(u256(-103)));
-	ABI_CHECK(callContractFunction("f(int32,int32)", u256(0xFFFFFF99u), u256(2)), encodeArgs(u256(-26)));
+	ABI_CHECK(callContractFunction("f(int32,int32)", u256(0xFFFFFF99u), u256(1)), v2 ? encodeArgs() : encodeArgs(u256(-52)));
-	ABI_CHECK(callContractFunction("f(int32,int32)", u256(0xFFFFFF99u), u256(4)), encodeArgs(u256(-7)));
+	ABI_CHECK(callContractFunction("f(int32,int32)", u256(0xFFFFFF99u), u256(2)), v2 ? encodeArgs() : encodeArgs(u256(-26)));
-	ABI_CHECK(callContractFunction("f(int32,int32)", u256(0xFFFFFF99u), u256(8)), encodeArgs(u256(-1)));
+	ABI_CHECK(callContractFunction("f(int32,int32)", u256(0xFFFFFF99u), u256(4)), v2 ? encodeArgs() : encodeArgs(u256(-7)));
+	ABI_CHECK(callContractFunction("f(int32,int32)", u256(0xFFFFFF99u), u256(8)), v2 ? encodeArgs() : encodeArgs(u256(-1)));
 }