Merge pull request #13380 from ethereum/multiple-indirections

Permit multiple indirections in coding calldata to and from memory/calldata

Author: bshastry

test/tools/ossfuzz/AbiV2IsabelleFuzzer.cpp

@@ -36,7 +36,7 @@ static evmc::VM evmone = evmc::VM{evmc_create_evmone()};
 
 DEFINE_PROTO_FUZZER(Contract const& _contract)
 {
-	ProtoConverter converter;
+	ProtoConverter converter(_contract.seed());
 	string contractSource = converter.contractToString(_contract);
 
 	if (const char* dump_path = getenv("PROTO_FUZZER_DUMP_PATH"))
@@ -69,14 +69,11 @@ DEFINE_PROTO_FUZZER(Contract const& _contract)
 			{}
 		);
 		auto result = evmoneUtil.compileDeployAndExecute(encodedData);
-		if (result.has_value())
-		{
-			solAssert(result->status_code != EVMC_REVERT, "Proto ABIv2 fuzzer: EVM One reverted.");
-			if (result->status_code == EVMC_SUCCESS)
-				solAssert(
-					EvmoneUtility::zeroWord(result->output_data, result->output_size),
-					"Proto ABIv2 fuzzer: ABIv2 coding failure found."
-				);
-		}
+		solAssert(result.status_code != EVMC_REVERT, "Proto ABIv2 fuzzer: EVM One reverted.");
+		if (result.status_code == EVMC_SUCCESS)
+			solAssert(
+				EvmoneUtility::zeroWord(result.output_data, result.output_size),
+				"Proto ABIv2 fuzzer: ABIv2 coding failure found."
+			);
 	}
 }

test/tools/ossfuzz/SolidityEvmoneInterface.cpp

@@ -128,59 +128,51 @@ evmc::result EvmoneUtility::deployAndExecute(
 	return callResult;
 }
 
-optional<evmc::result> EvmoneUtility::compileDeployAndExecute(string _fuzzIsabelle)
+evmc::result EvmoneUtility::compileDeployAndExecute(string _fuzzIsabelle)
 {
 	map<string, h160> libraryAddressMap;
 	// Stage 1: Compile and deploy library if present.
 	if (!m_libraryName.empty())
 	{
 		m_compilationFramework.contractName(m_libraryName);
 		auto compilationOutput = m_compilationFramework.compileContract();
-		if (compilationOutput.has_value())
-		{
-			CompilerOutput cOutput = compilationOutput.value();
-			// Deploy contract and signal failure if deploy failed
-			evmc::result createResult = deployContract(cOutput.byteCode);
-			solAssert(
-				createResult.status_code == EVMC_SUCCESS,
-				"SolidityEvmoneInterface: Library deployment failed"
-			);
-			libraryAddressMap[m_libraryName] = EVMHost::convertFromEVMC(createResult.create_address);
-			m_compilationFramework.libraryAddresses(libraryAddressMap);
-		}
-		else
-			return {};
+		solAssert(compilationOutput.has_value(), "Compiling library failed");
+		CompilerOutput cOutput = compilationOutput.value();
+		// Deploy contract and signal failure if deploy failed
+		evmc::result createResult = deployContract(cOutput.byteCode);
+		solAssert(
+			createResult.status_code == EVMC_SUCCESS,
+			"SolidityEvmoneInterface: Library deployment failed"
+		);
+		libraryAddressMap[m_libraryName] = EVMHost::convertFromEVMC(createResult.create_address);
+		m_compilationFramework.libraryAddresses(libraryAddressMap);
 	}
 
 	// Stage 2: Compile, deploy, and execute contract, optionally using library
 	// address map.
 	m_compilationFramework.contractName(m_contractName);
 	auto cOutput = m_compilationFramework.compileContract();
-	if (cOutput.has_value())
-	{
-		solAssert(
-			!cOutput->byteCode.empty() && !cOutput->methodIdentifiersInContract.empty(),
-			"SolidityEvmoneInterface: Invalid compilation output."
-		);
-
-		string methodName;
-		if (!_fuzzIsabelle.empty())
-			// TODO: Remove this once a cleaner solution is found for querying
-			// isabelle test entry point. At the moment, we are sure that the
-			// entry point is the second method in the contract (hence the ++)
-			// but not its name.
-			methodName = (++cOutput->methodIdentifiersInContract.begin())->asString() +
-				_fuzzIsabelle.substr(2, _fuzzIsabelle.size());
-		else
-			methodName = cOutput->methodIdentifiersInContract[m_methodName].asString();
+	solAssert(cOutput.has_value(), "Compiling contract failed");
+	solAssert(
+		!cOutput->byteCode.empty() && !cOutput->methodIdentifiersInContract.empty(),
+		"SolidityEvmoneInterface: Invalid compilation output."
+	);
 
-		return deployAndExecute(
-			cOutput->byteCode,
-			methodName
-		);
-	}
+	string methodName;
+	if (!_fuzzIsabelle.empty())
+		// TODO: Remove this once a cleaner solution is found for querying
+		// isabelle test entry point. At the moment, we are sure that the
+		// entry point is the second method in the contract (hence the ++)
+		// but not its name.
+		methodName = (++cOutput->methodIdentifiersInContract.begin())->asString() +
+			_fuzzIsabelle.substr(2, _fuzzIsabelle.size());
 	else
-		return {};
+		methodName = cOutput->methodIdentifiersInContract[m_methodName].asString();
+
+	return deployAndExecute(
+		cOutput->byteCode,
+		methodName
+	);
 }
 
 optional<CompilerOutput> EvmoneUtility::compileContract()

test/tools/ossfuzz/SolidityEvmoneInterface.h

@@ -122,7 +122,7 @@ class EvmoneUtility
 	/// and executing test configuration.
 	/// @param _isabelleData contains encoding data to be passed to the
 	/// isabelle test entry point.
-	std::optional<evmc::result> compileDeployAndExecute(std::string _isabelleData = {});
+	evmc::result compileDeployAndExecute(std::string _isabelleData = {});
 	/// Compares the contents of the memory address pointed to
 	/// by `_result` of `_length` bytes to u256 zero.
 	/// @returns true if `_result` is zero, false

test/tools/ossfuzz/abiV2Proto.proto

@@ -92,6 +92,7 @@ message Contract {
     required VarDecl state_vars = 1;
     required TestFunction testfunction = 2;
     required Test test = 3;
+    required uint32 seed = 4;
 }
 
 package solidity.test.abiv2fuzzer;

test/tools/ossfuzz/abiV2ProtoFuzzer.cpp

@@ -35,7 +35,7 @@ static evmc::VM evmone = evmc::VM{evmc_create_evmone()};
 
 DEFINE_PROTO_FUZZER(Contract const& _input)
 {
-	string contract_source = ProtoConverter{}.contractToString(_input);
+	string contract_source = ProtoConverter{_input.seed()}.contractToString(_input);
 
 	if (const char* dump_path = getenv("PROTO_FUZZER_DUMP_PATH"))
 	{
@@ -61,21 +61,18 @@ DEFINE_PROTO_FUZZER(Contract const& _input)
 	);
 	// Invoke test function
 	auto result = evmoneUtil.compileDeployAndExecute();
-	if (result.has_value())
-	{
-		// We don't care about EVM One failures other than EVMC_REVERT
-		solAssert(result->status_code != EVMC_REVERT, "Proto ABIv2 fuzzer: EVM One reverted");
-		if (result->status_code == EVMC_SUCCESS)
-			if (!EvmoneUtility::zeroWord(result->output_data, result->output_size))
-			{
-				solidity::bytes resultAsBytes;
-				for (size_t i = 0; i < result->output_size; i++)
-					resultAsBytes.push_back(result->output_data[i]);
-				cout << solidity::util::toHex(resultAsBytes) << endl;
-				solAssert(
-					false,
-					"Proto ABIv2 fuzzer: ABIv2 coding failure found"
-				);
-			}
-	}
+	// We don't care about EVM One failures other than EVMC_REVERT
+	solAssert(result.status_code != EVMC_REVERT, "Proto ABIv2 fuzzer: EVM One reverted");
+	if (result.status_code == EVMC_SUCCESS)
+		if (!EvmoneUtility::zeroWord(result.output_data, result.output_size))
+		{
+			solidity::bytes res;
+			for (size_t i = 0; i < result.output_size; i++)
+				res.push_back(result.output_data[i]);
+			cout << solidity::util::toHex(res) << endl;
+			solAssert(
+				false,
+				"Proto ABIv2 fuzzer: ABIv2 coding failure found"
+			);
+		}
 }

test/tools/ossfuzz/protoToAbiV2.cpp

@@ -416,16 +416,7 @@ void ProtoConverter::appendTypedParamsExternal(
     Delimiter _delimiter
 )
 {
-	std::string qualifiedTypeString = (
-		_isValueType ?
-		_typeString :
-		_typeString + " calldata"
-	);
-	m_typedParamsExternal << Whiskers(R"(<delimiter><type> <varName>)")
-		("delimiter", delimiterToString(_delimiter))
-		("type", qualifiedTypeString)
-		("varName", _varName)
-		.render();
+	m_externalParamsRep.push_back({_delimiter, _isValueType, _typeString, _varName});
 	m_untypedParamsExternal << Whiskers(R"(<delimiter><varName>)")
 		("delimiter", delimiterToString(_delimiter))
 		("varName", _varName)
@@ -475,7 +466,25 @@ std::string ProtoConverter::typedParametersAsString(CalleeType _calleeType)
 	case CalleeType::PUBLIC:
 		return m_typedParamsPublic.str();
 	case CalleeType::EXTERNAL:
-		return m_typedParamsExternal.str();
+	{
+		ostringstream typedParamsExternal;
+		for (auto const& i: m_externalParamsRep)
+		{
+			Delimiter del = get<0>(i);
+			bool valueType = get<1>(i);
+			string typeString = get<2>(i);
+			string varName = get<3>(i);
+			bool isCalldata = randomBool(/*probability=*/0.5);
+			string location = (isCalldata ? "calldata" : "memory");
+			string qualifiedTypeString = (valueType ? typeString : typeString + " " + location);
+			typedParamsExternal << Whiskers(R"(<delimiter><type> <varName>)")
+				("delimiter", delimiterToString(del))
+				("type", qualifiedTypeString)
+				("varName", varName)
+				.render();
+		}
+		return typedParamsExternal.str();
+	}
 	}
 }
 
@@ -666,6 +675,33 @@ string ProtoConverter::calldataHelperFunctions()
 		return 0;
 	})";
 
+	/// These are indirections to test memory-calldata codings more robustly.
+	stringstream indirections;
+	unsigned numIndirections = randomNumberOneToN(s_maxIndirections);
+	for (unsigned i = 1; i <= numIndirections; i++)
+	{
+		bool finalIndirection = i == numIndirections;
+		string mutability = (finalIndirection ? "pure" : "view");
+		indirections << Whiskers(R"(
+	function coder_calldata_external_i<N>(<parameters>) external <mutability> returns (uint) {
+<?finalIndirection>
+<equality_checks>
+		return 0;
+<!finalIndirection>
+		return this.coder_calldata_external_i<NPlusOne>(<untyped_parameters>);
+</finalIndirection>
+	}
+		)")
+		("N", to_string(i))
+		("parameters", typedParametersAsString(CalleeType::EXTERNAL))
+		("mutability", mutability)
+		("finalIndirection", finalIndirection)
+		("equality_checks", equalityChecksAsString())
+		("NPlusOne", to_string(i + 1))
+		("untyped_parameters", m_untypedParamsExternal.str())
+		.render();
+	}
+
 	// These are callee functions that encode from storage, decode to
 	// memory/calldata and check if decoded value matches storage value
 	// return true on successful match, false otherwise
@@ -676,18 +712,15 @@ string ProtoConverter::calldataHelperFunctions()
 	}
 
 	function coder_calldata_external(<parameters_calldata>) external view returns (uint) {
-		return this.coder_calldata_external_indirection(<untyped_parameters>);
-	}
-
-	function coder_calldata_external_indirection(<parameters_calldata>) external pure returns (uint) {
-<equality_checks>
-		return 0;
+		return this.coder_calldata_external_i1(<untyped_parameters>);
 	}
+<indirections>
 	)")
 	("parameters_memory", typedParametersAsString(CalleeType::PUBLIC))
 	("equality_checks", equalityChecksAsString())
 	("parameters_calldata", typedParametersAsString(CalleeType::EXTERNAL))
 	("untyped_parameters", m_untypedParamsExternal.str())
+	("indirections", indirections.str())
 	.render();
 
 	return calldataHelperFuncs.str();

test/tools/ossfuzz/protoToAbiV2.h

@@ -15,6 +15,7 @@
 #include <boost/variant.hpp>
 
 #include <ostream>
+#include <random>
 #include <sstream>
 
 /**
@@ -134,21 +135,26 @@
 
 namespace solidity::test::abiv2fuzzer
 {
+using RandomEngine = std::mt19937_64;
+using Distribution = std::uniform_int_distribution<unsigned>;
+using Bernoulli = std::bernoulli_distribution;
 
 /// Converts a protobuf input into a Solidity program that tests
 /// abi coding.
 class ProtoConverter
 {
 public:
-	ProtoConverter():
+	ProtoConverter(unsigned _seed):
 		m_isStateVar(true),
 		m_counter(0),
 		m_varCounter(0),
 		m_returnValue(1),
 		m_isLastDynParamRightPadded(false),
 		m_structCounter(0),
 		m_numStructsAdded(0)
-	{}
+	{
+		m_random = std::make_unique<RandomEngine>(_seed);
+	}
 
 	ProtoConverter(ProtoConverter const&) = delete;
 	ProtoConverter(ProtoConverter&&) = delete;
@@ -173,6 +179,13 @@ class ProtoConverter
 		EXTERNAL
 	};
 
+	/// Each external parameter representation contains the following:
+	/// - Delimiter prefix
+	/// - Boolean that is true if value type, false otherwise
+	/// - String representation of type
+	/// - Parameter name
+	using ParameterPack = std::tuple<Delimiter, bool, std::string, std::string>;
+
 	/// Visitors for various Protobuf types
 	/// Visit top-level contract specification
 	void visit(Contract const&);
@@ -381,14 +394,23 @@ class ProtoConverter
 
 	/// Convert delimter to a comma or null string.
 	static std::string delimiterToString(Delimiter _delimiter, bool _space = true);
+	/// Generates number in the range [1, @param _n] uniformly at random.
+	unsigned randomNumberOneToN(unsigned _n)
+	{
+		return Distribution(1, _n)(*m_random);
+	}
+	/// Generates boolean that has a bernoulli distribution defined by @param _p.
+	bool randomBool(double _p)
+	{
+		return Bernoulli{_p}(*m_random);
+	}
 
 	/// Contains the test program
 	std::ostringstream m_output;
 	/// Contains a subset of the test program. This subset contains
 	/// checks to be encoded in the test program
 	std::ostringstream m_checks;
 	/// Contains typed parameter list to be passed to callee functions
-	std::ostringstream m_typedParamsExternal;
 	std::ostringstream m_typedParamsPublic;
 	/// Contains parameter list to be passed to callee functions
 	std::ostringstream m_untypedParamsExternal;
@@ -418,10 +440,16 @@ class ProtoConverter
 	unsigned m_numStructsAdded;
 	/// Enum stating abiv2 coder to be tested
 	Contract_Test m_test;
+	/// Representation of external parameters
+	std::vector<ParameterPack> m_externalParamsRep;
+	/// Random number generator
+	std::unique_ptr<RandomEngine> m_random;
 	/// Prefixes for declared and parameterized variable names
 	static auto constexpr s_localVarNamePrefix = "lv_";
 	static auto constexpr s_stateVarNamePrefix = "sv_";
 	static auto constexpr s_paramNamePrefix = "p_";
+	/// Maximum number of indirections to test calldata coding
+	static unsigned constexpr s_maxIndirections = 5;
 };
 
 /// Visitor interface for Solidity protobuf types.

test/tools/ossfuzz/solProtoFuzzer.cpp

@@ -79,13 +79,10 @@ DEFINE_PROTO_FUZZER(Program const& _input)
 		methodName
 	);
 	auto minimalResult = evmoneUtil.compileDeployAndExecute();
-	if (minimalResult.has_value())
-	{
-		solAssert(minimalResult->status_code != EVMC_REVERT, "Sol proto fuzzer: Evmone reverted.");
-		if (minimalResult->status_code == EVMC_SUCCESS)
+	solAssert(minimalResult.status_code != EVMC_REVERT, "Sol proto fuzzer: Evmone reverted.");
+	if (minimalResult.status_code == EVMC_SUCCESS)
 		solAssert(
-			EvmoneUtility::zeroWord(minimalResult->output_data, minimalResult->output_size),
+			EvmoneUtility::zeroWord(minimalResult.output_data, minimalResult.output_size),
 			"Proto solc fuzzer: Output incorrect"
 		);
-	}
 }