Merge pull request #3203 from ethereum/nocall

Prevent libraries from being called.

Changelog.md

@@ -1,6 +1,7 @@
 ### 0.4.20 (unreleased)
 
 Features:
+ * Code Generator: Prevent non-view functions in libraries from being called directly.
  * Commandline interface: Support strict mode of assembly with the ``--strict--assembly`` switch.
  * Limit the number of warnings raised for creating abstract contracts.
  * Inline Assembly: Issue warning for using jump labels (already existed for jump instructions).

docs/contracts.rst

@@ -1100,7 +1100,11 @@ is executed in the context of the calling contract, i.e. ``this`` points to the
 calling contract, and especially the storage from the calling contract can be
 accessed. As a library is an isolated piece of source code, it can only access
 state variables of the calling contract if they are explicitly supplied (it
-would have no way to name them, otherwise).
+would have no way to name them, otherwise). Library functions can only be
+called directly (i.e. without the use of ``DELEGATECALL``) if they do not modify
+the state (i.e. if they are ``view`` or ``pure`` functions),
+because libraries are assumed to be stateless. In particular, it is
+not possible to destroy a library unless Solidity's type system is circumvented.
 
 Libraries can be seen as implicit base contracts of the contracts that use them.
 They will not be explicitly visible in the inheritance hierarchy, but calls
@@ -1268,6 +1272,30 @@ Restrictions for libraries in comparison to contracts:
 
 (These might be lifted at a later point.)
 
+Call Protection For Libraries
+=============================
+
+As mentioned in the introduction, if a library's code is executed
+using a ``CALL`` instead of a ``DELEGATECALL`` or ``CALLCODE``,
+it will revert unless a ``view`` or ``pure`` function is called.
+
+The EVM does not provide a direct way for a contract to detect
+whether it was called using ``CALL`` or not, but a contract
+can use the ``ADDRESS`` opcode to find out "where" it is
+currently running. The generated code compares this address
+to the address used at construction time to determine the mode
+of calling.
+
+More specifically, the runtime code of a library always starts
+with a push instruction, which is a zero of 20 bytes at
+compilation time. When the deploy code runs, this constant
+is replaced in memory by the current address and this
+modified code is stored in the contract. At runtime,
+this causes the deploy time address to be the first
+constant to be pushed onto the stack and the dispatcher
+code compares the current address against this constant
+for any non-view and non-pure function.
+
 .. index:: ! using for, library
 
 .. _using-for:

libevmasm/Assembly.cpp

@@ -283,6 +283,11 @@ Json::Value Assembly::assemblyJSON(StringMap const& _sourceCodes) const
 				createJsonValue("PUSHLIB", i.location().start, i.location().end, m_libraries.at(h256(i.data())))
 			);
 			break;
+		case PushDeployTimeAddress:
+			collection.append(
+				createJsonValue("PUSHDEPLOYADDRESS", i.location().start, i.location().end)
+			);
+			break;
 		case Tag:
 			collection.append(
 				createJsonValue("tag", i.location().start, i.location().end, string(i.data())));
@@ -590,6 +595,10 @@ LinkerObject const& Assembly::assemble() const
 			ret.linkReferences[ret.bytecode.size()] = m_libraries.at(i.data());
 			ret.bytecode.resize(ret.bytecode.size() + 20);
 			break;
+		case PushDeployTimeAddress:
+			ret.bytecode.push_back(byte(Instruction::PUSH20));
+			ret.bytecode.resize(ret.bytecode.size() + 20);
+			break;
 		case Tag:
 			assertThrow(i.data() != 0, AssemblyException, "Invalid tag position.");
 			assertThrow(i.splitForeignPushTag().first == size_t(-1), AssemblyException, "Foreign tag.");

libevmasm/AssemblyItem.cpp

@@ -68,6 +68,7 @@ unsigned AssemblyItem::bytesRequired(unsigned _addressLength) const
 	case PushSub:
 		return 1 + _addressLength;
 	case PushLibraryAddress:
+	case PushDeployTimeAddress:
 		return 1 + 20;
 	default:
 		break;
@@ -97,6 +98,7 @@ int AssemblyItem::returnValues() const
 	case PushSubSize:
 	case PushProgramSize:
 	case PushLibraryAddress:
+	case PushDeployTimeAddress:
 		return 1;
 	case Tag:
 		return 0;
@@ -119,6 +121,7 @@ bool AssemblyItem::canBeFunctional() const
 	case PushSubSize:
 	case PushProgramSize:
 	case PushLibraryAddress:
+	case PushDeployTimeAddress:
 		return true;
 	case Tag:
 		return false;
@@ -190,6 +193,9 @@ string AssemblyItem::toAssemblyText() const
 	case PushLibraryAddress:
 		text = string("linkerSymbol(\"") + toHex(data()) + string("\")");
 		break;
+	case PushDeployTimeAddress:
+		text = string("deployTimeAddress()");
+		break;
 	case UndefinedItem:
 		assertThrow(false, AssemblyException, "Invalid assembly item.");
 		break;
@@ -252,6 +258,9 @@ ostream& dev::eth::operator<<(ostream& _out, AssemblyItem const& _item)
 		_out << " PushLibraryAddress " << hash.substr(0, 8) + "..." + hash.substr(hash.length() - 8);
 		break;
 	}
+	case PushDeployTimeAddress:
+		_out << " PushDeployTimeAddress";
+		break;
 	case UndefinedItem:
 		_out << " ???";
 		break;

libevmasm/AssemblyItem.h

@@ -46,7 +46,8 @@ enum AssemblyItemType {
 	PushProgramSize,
 	Tag,
 	PushData,
-	PushLibraryAddress ///< Push a currently unknown address of another (library) contract.
+	PushLibraryAddress, ///< Push a currently unknown address of another (library) contract.
+	PushDeployTimeAddress ///< Push an address to be filled at deploy time. Should not be touched by the optimizer.
 };
 
 class Assembly;

libevmasm/GasMeter.cpp

@@ -52,6 +52,7 @@ GasMeter::GasConsumption GasMeter::estimateMax(AssemblyItem const& _item, bool _
 	case PushSubSize:
 	case PushProgramSize:
 	case PushLibraryAddress:
+	case PushDeployTimeAddress:
 		gas = runGas(Instruction::PUSH1);
 		break;
 	case Tag:

libevmasm/SemanticInformation.cpp

@@ -35,6 +35,7 @@ bool SemanticInformation::breaksCSEAnalysisBlock(AssemblyItem const& _item)
 	default:
 	case UndefinedItem:
 	case Tag:
+	case PushDeployTimeAddress:
 		return true;
 	case Push:
 	case PushString:

libsolidity/codegen/Compiler.cpp

@@ -51,6 +51,7 @@ void Compiler::compileClone(
 	map<ContractDefinition const*, eth::Assembly const*> const& _contracts
 )
 {
+	solAssert(!_contract.isLibrary(), "");
 	ContractCompiler runtimeCompiler(nullptr, m_runtimeContext, m_optimize);
 	ContractCompiler cloneCompiler(&runtimeCompiler, m_context, m_optimize);
 	m_runtimeSub = cloneCompiler.compileClone(_contract, _contracts);

libsolidity/codegen/CompilerContext.h

@@ -174,6 +174,9 @@ public:
 	eth::AssemblyItem appendData(bytes const& _data) { return m_asm->append(_data); }
 	/// Appends the address (virtual, will be filled in by linker) of a library.
 	void appendLibraryAddress(std::string const& _identifier) { m_asm->appendLibraryAddress(_identifier); }
+	/// Appends a zero-address that can be replaced by something else at deploy time (if the
+	/// position in bytecode is known).
+	void appendDeployTimeAddress() { m_asm->append(eth::PushDeployTimeAddress); }
 	/// Resets the stack of visited nodes with a new stack having only @c _node
 	void resetVisitedNodes(ASTNode const* _node);
 	/// Pops the stack of visited nodes

libsolidity/codegen/ContractCompiler.cpp

@@ -64,6 +64,12 @@ void ContractCompiler::compileContract(
 )
 {
 	CompilerContext::LocationSetter locationSetter(m_context, _contract);
+
+	if (_contract.isLibrary())
+		// Check whether this is a call (true) or a delegatecall (false).
+		// This has to be the first code in the contract.
+		appendDelegatecallCheck();
+
 	initializeContext(_contract, _contracts);
 	appendFunctionSelector(_contract);
 	appendMissingFunctions();
@@ -75,8 +81,13 @@ size_t ContractCompiler::compileConstructor(
 )
 {
 	CompilerContext::LocationSetter locationSetter(m_context, _contract);
-	initializeContext(_contract, _contracts);
-	return packIntoContractCreator(_contract);
+	if (_contract.isLibrary())
+		return deployLibrary(_contract);
+	else
+	{
+		initializeContext(_contract, _contracts);
+		return packIntoContractCreator(_contract);
+	}
 }
 
 size_t ContractCompiler::compileClone(
@@ -122,6 +133,7 @@ void ContractCompiler::appendCallValueCheck()
 
 void ContractCompiler::appendInitAndConstructorCode(ContractDefinition const& _contract)
 {
+	solAssert(!_contract.isLibrary(), "Tried to initialize library.");
 	CompilerContext::LocationSetter locationSetter(m_context, _contract);
 	// Determine the arguments that are used for the base constructors.
 	std::vector<ContractDefinition const*> const& bases = _contract.annotation().linearizedBaseContracts;
@@ -163,6 +175,7 @@ void ContractCompiler::appendInitAndConstructorCode(ContractDefinition const& _c
 size_t ContractCompiler::packIntoContractCreator(ContractDefinition const& _contract)
 {
 	solAssert(!!m_runtimeCompiler, "");
+	solAssert(!_contract.isLibrary(), "Tried to use contract creator or library.");
 
 	appendInitAndConstructorCode(_contract);
 
@@ -188,6 +201,34 @@ size_t ContractCompiler::packIntoContractCreator(ContractDefinition const& _cont
 	return m_context.runtimeSub();
 }
 
+size_t ContractCompiler::deployLibrary(ContractDefinition const& _contract)
+{
+	solAssert(!!m_runtimeCompiler, "");
+	solAssert(_contract.isLibrary(), "Tried to deploy contract as library.");
+
+	CompilerContext::LocationSetter locationSetter(m_context, _contract);
+
+	solAssert(m_context.runtimeSub() != size_t(-1), "Runtime sub not registered");
+	m_context.pushSubroutineSize(m_context.runtimeSub());
+	m_context.pushSubroutineOffset(m_context.runtimeSub());
+	m_context.appendInlineAssembly(R"(
+	{
+		// If code starts at 11, an mstore(0) writes to the full PUSH20 plus data
+		// without the need for a shift.
+		let codepos := 11
+		codecopy(codepos, subOffset, subSize)
+		// Check that the first opcode is a PUSH20
+		switch eq(0x73, byte(0, mload(codepos)))
+		case 0 { invalid() }
+		mstore(0, address())
+		mstore8(codepos, 0x73)
+		return(codepos, subSize)
+	}
+	)", {"subSize", "subOffset"});
+
+	return m_context.runtimeSub();
+}
+
 void ContractCompiler::appendBaseConstructor(FunctionDefinition const& _constructor)
 {
 	CompilerContext::LocationSetter locationSetter(m_context, _constructor);
@@ -244,11 +285,26 @@ void ContractCompiler::appendConstructor(FunctionDefinition const& _constructor)
 	_constructor.accept(*this);
 }
 
+void ContractCompiler::appendDelegatecallCheck()
+{
+	// Special constant that will be replaced by the address at deploy time.
+	// At compilation time, this is just "PUSH20 00...000".
+	m_context.appendDeployTimeAddress();
+	m_context << Instruction::ADDRESS << Instruction::EQ;
+	// The result on the stack is
+	// "We have not been called via DELEGATECALL".
+}
+
 void ContractCompiler::appendFunctionSelector(ContractDefinition const& _contract)
 {
 	map<FixedHash<4>, FunctionTypePointer> interfaceFunctions = _contract.interfaceFunctions();
 	map<FixedHash<4>, const eth::AssemblyItem> callDataUnpackerEntryPoints;
 
+	if (_contract.isLibrary())
+	{
+		solAssert(m_context.stackHeight() == 1, "CALL / DELEGATECALL flag expected.");
+	}
+
 	FunctionDefinition const* fallback = _contract.fallbackFunction();
 	eth::AssemblyItem notFound = m_context.newTag();
 	// directly jump to fallback if the data is too short to contain a function selector
@@ -260,7 +316,7 @@ void ContractCompiler::appendFunctionSelector(ContractDefinition const& _contrac
 	if (!interfaceFunctions.empty())
 		CompilerUtils(m_context).loadFromMemory(0, IntegerType(CompilerUtils::dataStartOffset * 8), true);
 
-	// stack now is: 1 0 <funhash>
+	// stack now is: <can-call-non-view-functions>? <funhash>
 	for (auto const& it: interfaceFunctions)
 	{
 		callDataUnpackerEntryPoints.insert(std::make_pair(it.first, m_context.newTag()));
@@ -272,6 +328,7 @@ void ContractCompiler::appendFunctionSelector(ContractDefinition const& _contrac
 	m_context << notFound;
 	if (fallback)
 	{
+		solAssert(!_contract.isLibrary(), "");
 		if (!fallback->isPayable())
 			appendCallValueCheck();
 
@@ -291,6 +348,13 @@ void ContractCompiler::appendFunctionSelector(ContractDefinition const& _contrac
 		CompilerContext::LocationSetter locationSetter(m_context, functionType->declaration());
 
 		m_context << callDataUnpackerEntryPoints.at(it.first);
+		if (_contract.isLibrary() && functionType->stateMutability() > StateMutability::View)
+		{
+			// If the function is not a view function and is called without DELEGATECALL,
+			// we revert.
+			m_context << dupInstruction(2);
+			m_context.appendConditionalRevert();
+		}
 		m_context.setStackOffset(0);
 		// We have to allow this for libraries, because value of the previous
 		// call is still visible in the delegatecall.
@@ -441,6 +505,7 @@ void ContractCompiler::registerStateVariables(ContractDefinition const& _contrac
 
 void ContractCompiler::initializeStateVariables(ContractDefinition const& _contract)
 {
+	solAssert(!_contract.isLibrary(), "Tried to initialize state variables of library.");
 	for (VariableDeclaration const* variable: _contract.stateVariables())
 		if (variable->value() && !variable->isConstant())
 			ExpressionCompiler(m_context, m_optimise).appendStateVariableInitialization(*variable);

libsolidity/codegen/ContractCompiler.h

@@ -75,10 +75,19 @@ private:
 	/// with a new and initialized context. Adds the constructor code.
 	/// @returns the identifier of the runtime sub assembly
 	size_t packIntoContractCreator(ContractDefinition const& _contract);
+	/// Appends code that deploys the given contract as a library.
+	/// Will also add code that modifies the contract in memory by injecting the current address
+	/// for the call protector.
+	size_t deployLibrary(ContractDefinition const& _contract);
 	/// Appends state variable initialisation and constructor code.
 	void appendInitAndConstructorCode(ContractDefinition const& _contract);
 	void appendBaseConstructor(FunctionDefinition const& _constructor);
 	void appendConstructor(FunctionDefinition const& _constructor);
+	/// Appends code that returns a boolean flag on the stack that tells whether
+	/// the contract has been called via delegatecall (false) or regular call (true).
+	/// This is done by inserting a specific push constant as the first instruction
+	/// whose data will be modified in memory at deploy time.
+	void appendDelegatecallCheck();
 	void appendFunctionSelector(ContractDefinition const& _contract);
 	void appendCallValueCheck();
 	/// Creates code that unpacks the arguments for the given function represented by a vector of TypePointers.

libsolidity/interface/CompilerStack.cpp

@@ -734,9 +734,12 @@ void CompilerStack::compileContract(
 
 	try
 	{
-		Compiler cloneCompiler(m_optimize, m_optimizeRuns);
-		cloneCompiler.compileClone(_contract, _compiledContracts);
-		compiledContract.cloneObject = cloneCompiler.assembledObject();
+		if (!_contract.isLibrary())
+		{
+			Compiler cloneCompiler(m_optimize, m_optimizeRuns);
+			cloneCompiler.compileClone(_contract, _compiledContracts);
+			compiledContract.cloneObject = cloneCompiler.assembledObject();
+		}
 	}
 	catch (eth::AssemblyException const&)
 	{

test/libsolidity/SolidityEndToEndTest.cpp

@@ -3543,6 +3543,39 @@ BOOST_AUTO_TEST_CASE(library_call_in_homestead)
 	ABI_CHECK(callContractFunction("sender()"), encodeArgs(u160(m_sender)));
 }
 
+BOOST_AUTO_TEST_CASE(library_call_protection)
+{
+	// This tests code that reverts a call if it is a direct call to a library
+	// as opposed to a delegatecall.
+	char const* sourceCode = R"(
+		library Lib {
+			struct S { uint x; }
+			// a direct call to this should revert
+			function np(S storage s) public returns (address) { s.x = 3; return msg.sender; }
+			// a direct call to this is fine
+			function v(S storage) public view returns (address) { return msg.sender; }
+			// a direct call to this is fine
+			function pu() public pure returns (uint) { return 2; }
+		}
+		contract Test {
+			Lib.S public s;
+			function np() public returns (address) { return Lib.np(s); }
+			function v() public view returns (address) { return Lib.v(s); }
+			function pu() public pure returns (uint) { return Lib.pu(); }
+		}
+	)";
+	compileAndRun(sourceCode, 0, "Lib");
+	ABI_CHECK(callContractFunction("np(Lib.S storage)"), encodeArgs());
+	ABI_CHECK(callContractFunction("v(Lib.S storage)"), encodeArgs(u160(m_sender)));
+	ABI_CHECK(callContractFunction("pu()"), encodeArgs(2));
+	compileAndRun(sourceCode, 0, "Test", bytes(), map<string, Address>{{"Lib", m_contractAddress}});
+	ABI_CHECK(callContractFunction("s()"), encodeArgs(0));
+	ABI_CHECK(callContractFunction("np()"), encodeArgs(u160(m_sender)));
+	ABI_CHECK(callContractFunction("s()"), encodeArgs(3));
+	ABI_CHECK(callContractFunction("v()"), encodeArgs(u160(m_sender)));
+	ABI_CHECK(callContractFunction("pu()"), encodeArgs(2));
+}
+
 BOOST_AUTO_TEST_CASE(store_bytes)
 {
 	// this test just checks that the copy loop does not mess up the stack