Merge pull request #3065 from ethereum/reject_truncated_selectors

Do not accept truncated function selectors.

7 files changed, 76 insertions, 12 deletions

diff --git a/Changelog.md b/Changelog.md
index ed3cdfea..c9e76ee5 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -4,6 +4,8 @@ Features:
  * Code Generator: Always use all available gas for calls as experimental 0.5.0 feature
    (previously, some amount was retained in order to work in pre-tangerine whistle
    EVM versions)
+ * Code Generator: Do not accept data with less than four bytes (truncated function
+   signature) for regular function calls - fallback function is invoked instead.
  * Parser: Better error message for unexpected trailing comma in parameter lists.
  * Standard JSON: Support the ``outputSelection`` field for selective compilation of supplied sources.
  * Syntax Checker: Unary ``+`` is now a syntax error as experimental 0.5.0 feature.
diff --git a/docs/bugs.json b/docs/bugs.json
index ac322a48..c642793a 100644
--- a/docs/bugs.json
+++ b/docs/bugs.json
@@ -1,5 +1,12 @@
 [
     {
+        "name": "ZeroFunctionSelector",
+        "summary": "It is possible to craft the name of a function such that it is executed instead of the fallback function in very specific circumstances.",
+        "description": "If a function has a selector consisting only of zeros, is payable and part of a contract that does not have a fallback function and at most five external functions in total, this function is called instead of the fallback function if Ether is sent to the contract without data.",
+        "fixed": "0.4.18",
+        "severity": "very low"
+    },
+    {
         "name": "DelegateCallReturnValue",
         "summary": "The low-level .delegatecall() does not return the execution outcome, but converts the value returned by the functioned called to a boolean instead.",
         "description": "The return value of the low-level .delegatecall() function is taken from a position in memory, where the call data or the return data resides. This value is interpreted as a boolean and put onto the stack. This means if the called function returns at least 32 zero bytes, .delegatecall() returns false even if the call was successuful.",
diff --git a/docs/bugs.rst b/docs/bugs.rst
index 55771a35..7629830d 100644
--- a/docs/bugs.rst
+++ b/docs/bugs.rst
@@ -48,7 +48,7 @@ fixed
 publish
     The date at which the bug became known publicly, optional
 severity
-    Severity of the bug: low, medium, high. Takes into account
+    Severity of the bug: very low, low, medium, high. Takes into account
     discoverability in contract tests, likelihood of occurrence and
     potential damage by exploits.
 conditions
diff --git a/docs/bugs_by_version.json b/docs/bugs_by_version.json
index c3686ebf..48881d0c 100644
--- a/docs/bugs_by_version.json
+++ b/docs/bugs_by_version.json
@@ -1,6 +1,7 @@
 {
     "0.1.0": {
         "bugs": [
+            "ZeroFunctionSelector", 
             "ECRecoverMalformedInput", 
             "SkipEmptyStringLiteral", 
             "ConstantOptimizerSubtraction", 
@@ -17,6 +18,7 @@
     }, 
     "0.1.1": {
         "bugs": [
+            "ZeroFunctionSelector", 
             "ECRecoverMalformedInput", 
             "SkipEmptyStringLiteral", 
             "ConstantOptimizerSubtraction", 
@@ -33,6 +35,7 @@
     }, 
     "0.1.2": {
         "bugs": [
+            "ZeroFunctionSelector", 
             "ECRecoverMalformedInput", 
             "SkipEmptyStringLiteral", 
             "ConstantOptimizerSubtraction", 
@@ -49,6 +52,7 @@
     }, 
     "0.1.3": {
         "bugs": [
+            "ZeroFunctionSelector", 
             "ECRecoverMalformedInput", 
             "SkipEmptyStringLiteral", 
             "ConstantOptimizerSubtraction", 
@@ -65,6 +69,7 @@
     }, 
     "0.1.4": {
         "bugs": [
+            "ZeroFunctionSelector", 
             "ECRecoverMalformedInput", 
             "SkipEmptyStringLiteral", 
             "ConstantOptimizerSubtraction", 
@@ -81,6 +86,7 @@
     }, 
     "0.1.5": {
         "bugs": [
+            "ZeroFunctionSelector", 
             "ECRecoverMalformedInput", 
             "SkipEmptyStringLiteral", 
             "ConstantOptimizerSubtraction", 
@@ -97,6 +103,7 @@
     }, 
     "0.1.6": {
         "bugs": [
+            "ZeroFunctionSelector", 
             "ECRecoverMalformedInput", 
             "SkipEmptyStringLiteral", 
             "ConstantOptimizerSubtraction", 
@@ -114,6 +121,7 @@
     }, 
     "0.1.7": {
         "bugs": [
+            "ZeroFunctionSelector", 
             "ECRecoverMalformedInput", 
             "SkipEmptyStringLiteral", 
             "ConstantOptimizerSubtraction", 
@@ -131,6 +139,7 @@
     }, 
     "0.2.0": {
         "bugs": [
+            "ZeroFunctionSelector", 
             "ECRecoverMalformedInput", 
             "SkipEmptyStringLiteral", 
             "ConstantOptimizerSubtraction", 
@@ -148,6 +157,7 @@
     }, 
     "0.2.1": {
         "bugs": [
+            "ZeroFunctionSelector", 
             "ECRecoverMalformedInput", 
             "SkipEmptyStringLiteral", 
             "ConstantOptimizerSubtraction", 
@@ -165,6 +175,7 @@
     }, 
     "0.2.2": {
         "bugs": [
+            "ZeroFunctionSelector", 
             "ECRecoverMalformedInput", 
             "SkipEmptyStringLiteral", 
             "ConstantOptimizerSubtraction", 
@@ -182,6 +193,7 @@
     }, 
     "0.3.0": {
         "bugs": [
+            "ZeroFunctionSelector", 
             "DelegateCallReturnValue", 
             "ECRecoverMalformedInput", 
             "SkipEmptyStringLiteral", 
@@ -199,6 +211,7 @@
     }, 
     "0.3.1": {
         "bugs": [
+            "ZeroFunctionSelector", 
             "DelegateCallReturnValue", 
             "ECRecoverMalformedInput", 
             "SkipEmptyStringLiteral", 
@@ -215,6 +228,7 @@
     }, 
     "0.3.2": {
         "bugs": [
+            "ZeroFunctionSelector", 
             "DelegateCallReturnValue", 
             "ECRecoverMalformedInput", 
             "SkipEmptyStringLiteral", 
@@ -231,6 +245,7 @@
     }, 
     "0.3.3": {
         "bugs": [
+            "ZeroFunctionSelector", 
             "DelegateCallReturnValue", 
             "ECRecoverMalformedInput", 
             "SkipEmptyStringLiteral", 
@@ -246,6 +261,7 @@
     }, 
     "0.3.4": {
         "bugs": [
+            "ZeroFunctionSelector", 
             "DelegateCallReturnValue", 
             "ECRecoverMalformedInput", 
             "SkipEmptyStringLiteral", 
@@ -261,6 +277,7 @@
     }, 
     "0.3.5": {
         "bugs": [
+            "ZeroFunctionSelector", 
             "DelegateCallReturnValue", 
             "ECRecoverMalformedInput", 
             "SkipEmptyStringLiteral", 
@@ -276,6 +293,7 @@
     }, 
     "0.3.6": {
         "bugs": [
+            "ZeroFunctionSelector", 
             "DelegateCallReturnValue", 
             "ECRecoverMalformedInput", 
             "SkipEmptyStringLiteral", 
@@ -289,6 +307,7 @@
     }, 
     "0.4.0": {
         "bugs": [
+            "ZeroFunctionSelector", 
             "DelegateCallReturnValue", 
             "ECRecoverMalformedInput", 
             "SkipEmptyStringLiteral", 
@@ -302,6 +321,7 @@
     }, 
     "0.4.1": {
         "bugs": [
+            "ZeroFunctionSelector", 
             "DelegateCallReturnValue", 
             "ECRecoverMalformedInput", 
             "SkipEmptyStringLiteral", 
@@ -315,6 +335,7 @@
     }, 
     "0.4.10": {
         "bugs": [
+            "ZeroFunctionSelector", 
             "DelegateCallReturnValue", 
             "ECRecoverMalformedInput", 
             "SkipEmptyStringLiteral", 
@@ -324,6 +345,7 @@
     }, 
     "0.4.11": {
         "bugs": [
+            "ZeroFunctionSelector", 
             "DelegateCallReturnValue", 
             "ECRecoverMalformedInput", 
             "SkipEmptyStringLiteral"
@@ -332,6 +354,7 @@
     }, 
     "0.4.12": {
         "bugs": [
+            "ZeroFunctionSelector", 
             "DelegateCallReturnValue", 
             "ECRecoverMalformedInput"
         ], 
@@ -339,6 +362,7 @@
     }, 
     "0.4.13": {
         "bugs": [
+            "ZeroFunctionSelector", 
             "DelegateCallReturnValue", 
             "ECRecoverMalformedInput"
         ], 
@@ -346,24 +370,32 @@
     }, 
     "0.4.14": {
         "bugs": [
+            "ZeroFunctionSelector", 
             "DelegateCallReturnValue"
         ], 
         "released": "2017-07-31"
     }, 
     "0.4.15": {
-        "bugs": [], 
+        "bugs": [
+            "ZeroFunctionSelector"
+        ], 
         "released": "2017-08-08"
     }, 
     "0.4.16": {
-        "bugs": [], 
+        "bugs": [
+            "ZeroFunctionSelector"
+        ], 
         "released": "2017-08-24"
     }, 
     "0.4.17": {
-        "bugs": [], 
+        "bugs": [
+            "ZeroFunctionSelector"
+        ], 
         "released": "2017-09-21"
     }, 
     "0.4.2": {
         "bugs": [
+            "ZeroFunctionSelector", 
             "DelegateCallReturnValue", 
             "ECRecoverMalformedInput", 
             "SkipEmptyStringLiteral", 
@@ -376,6 +408,7 @@
     }, 
     "0.4.3": {
         "bugs": [
+            "ZeroFunctionSelector", 
             "DelegateCallReturnValue", 
             "ECRecoverMalformedInput", 
             "SkipEmptyStringLiteral", 
@@ -387,6 +420,7 @@
     }, 
     "0.4.4": {
         "bugs": [
+            "ZeroFunctionSelector", 
             "DelegateCallReturnValue", 
             "ECRecoverMalformedInput", 
             "SkipEmptyStringLiteral", 
@@ -397,6 +431,7 @@
     }, 
     "0.4.5": {
         "bugs": [
+            "ZeroFunctionSelector", 
             "DelegateCallReturnValue", 
             "ECRecoverMalformedInput", 
             "SkipEmptyStringLiteral", 
@@ -408,6 +443,7 @@
     }, 
     "0.4.6": {
         "bugs": [
+            "ZeroFunctionSelector", 
             "DelegateCallReturnValue", 
             "ECRecoverMalformedInput", 
             "SkipEmptyStringLiteral", 
@@ -418,6 +454,7 @@
     }, 
     "0.4.7": {
         "bugs": [
+            "ZeroFunctionSelector", 
             "DelegateCallReturnValue", 
             "ECRecoverMalformedInput", 
             "SkipEmptyStringLiteral", 
@@ -427,6 +464,7 @@
     }, 
     "0.4.8": {
         "bugs": [
+            "ZeroFunctionSelector", 
             "DelegateCallReturnValue", 
             "ECRecoverMalformedInput", 
             "SkipEmptyStringLiteral", 
@@ -436,6 +474,7 @@
     }, 
     "0.4.9": {
         "bugs": [
+            "ZeroFunctionSelector", 
             "DelegateCallReturnValue", 
             "ECRecoverMalformedInput", 
             "SkipEmptyStringLiteral", 
diff --git a/libsolidity/codegen/ContractCompiler.cpp b/libsolidity/codegen/ContractCompiler.cpp
index 429db532..74565ae4 100644
--- a/libsolidity/codegen/ContractCompiler.cpp
+++ b/libsolidity/codegen/ContractCompiler.cpp
@@ -251,13 +251,10 @@ void ContractCompiler::appendFunctionSelector(ContractDefinition const& _contrac
 
 	FunctionDefinition const* fallback = _contract.fallbackFunction();
 	eth::AssemblyItem notFound = m_context.newTag();
-	// shortcut messages without data if we have many functions in order to be able to receive
-	// ether with constant gas
-	if (interfaceFunctions.size() > 5 || fallback)
-	{
-		m_context << Instruction::CALLDATASIZE << Instruction::ISZERO;
-		m_context.appendConditionalJumpTo(notFound);
-	}
+	// directly jump to fallback if the data is too short to contain a function selector
+	// also guards against short data
+	m_context << u256(4) << Instruction::CALLDATASIZE << Instruction::LT;
+	m_context.appendConditionalJumpTo(notFound);
 
 	// retrieve the function signature hash from the calldata
 	if (!interfaceFunctions.empty())
diff --git a/test/libsolidity/Assembly.cpp b/test/libsolidity/Assembly.cpp
index 56ac8cf5..358d3c72 100644
--- a/test/libsolidity/Assembly.cpp
+++ b/test/libsolidity/Assembly.cpp
@@ -119,7 +119,7 @@ BOOST_AUTO_TEST_CASE(location_test)
 	shared_ptr<string const> n = make_shared<string>("");
 	AssemblyItems items = compileContract(sourceCode);
 	vector<SourceLocation> locations =
-		vector<SourceLocation>(18, SourceLocation(2, 75, n)) +
+		vector<SourceLocation>(24, SourceLocation(2, 75, n)) +
 		vector<SourceLocation>(32, SourceLocation(20, 72, n)) +
 		vector<SourceLocation>{SourceLocation(42, 51, n), SourceLocation(65, 67, n)} +
 		vector<SourceLocation>(2, SourceLocation(58, 67, n)) +
diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp
index 648c13cb..9a837113 100644
--- a/test/libsolidity/SolidityEndToEndTest.cpp
+++ b/test/libsolidity/SolidityEndToEndTest.cpp
@@ -2899,6 +2899,25 @@ BOOST_AUTO_TEST_CASE(default_fallback_throws)
 	ABI_CHECK(callContractFunction("f()"), encodeArgs(0));
 }
 
+BOOST_AUTO_TEST_CASE(short_data_calls_fallback)
+{
+	char const* sourceCode = R"(
+		contract A {
+			uint public x;
+			// Signature is d88e0b00
+			function fow() { x = 3; }
+			function () { x = 2; }
+		}
+	)";
+	compileAndRun(sourceCode);
+	// should call fallback
+	sendMessage(asBytes("\xd8\x8e\x0b"), false, 0);
+	ABI_CHECK(callContractFunction("x()"), encodeArgs(2));
+	// should call function
+	sendMessage(asBytes(string("\xd8\x8e\x0b") + string(1, 0)), false, 0);
+	ABI_CHECK(callContractFunction("x()"), encodeArgs(3));
+}
+
 BOOST_AUTO_TEST_CASE(event)
 {
 	char const* sourceCode = R"(