Merge pull request #4138 from ethereum/warnVarArgs

Warn when hash functions are used with var arguments

16 files changed, 129 insertions, 10 deletions

diff --git a/Changelog.md b/Changelog.md
index 72c27f8e..ba7f4544 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -15,6 +15,7 @@ Features:
  * Type Checker: Deprecate the ``years`` unit denomination and raise a warning for it (or an error as experimental 0.5.0 feature).
  * Type Checker: Make literals (without explicit type casting) an error for tight packing as experimental 0.5.0 feature.
  * Type Checker: Warn about wildcard tuple assignments (this will turn into an error with version 0.5.0).
+ * Type Checker: Warn when ``keccak256``, ``sha256`` and ``ripemd160`` are not used with a single bytes argument (suggest to use ``abi.encodePacked(...)``). This will turn into an error with version 0.5.0.
 
 Bugfixes:
  * Code Generator: Fix ``revert`` with reason coming from a state or local string variable.
diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp
index e8694e88..f77cc60c 100644
--- a/libsolidity/analysis/TypeChecker.cpp
+++ b/libsolidity/analysis/TypeChecker.cpp
@@ -1760,6 +1760,33 @@ bool TypeChecker::visit(FunctionCall const& _functionCall)
 		}
 	}
 
+	if (functionType->takesSinglePackedBytesParameter())
+	{
+		string generalMessage =
+			"This function only accepts a single \"bytes\" argument. Please use "
+			"\"abi.encodePacked(...)\" or a similar function to encode the data.";
+
+		if (arguments.size() > 1)
+		{
+			if (v050)
+				m_errorReporter.typeError(_functionCall.location(), generalMessage);
+			else
+				m_errorReporter.warning(_functionCall.location(), generalMessage);
+		}
+		else if (arguments.size() == 1 && !type(*arguments.front())->isImplicitlyConvertibleTo(ArrayType(DataLocation::Memory)))
+		{
+			string msg =
+				generalMessage +
+				" The provided argument of type " +
+				type(*arguments.front())->toString() +
+				" is not implicitly convertible to expected type bytes memory.";
+			if (v050)
+				m_errorReporter.typeError(_functionCall.location(), msg);
+			else
+				m_errorReporter.warning(_functionCall.location(), msg);
+		}
+	}
+
 	if (functionType->takesArbitraryParameters() && arguments.size() < parameterTypes.size())
 	{
 		solAssert(_functionCall.annotation().kind == FunctionCallKind::FunctionCall, "");
diff --git a/libsolidity/ast/Types.h b/libsolidity/ast/Types.h
index 4884696d..95821634 100644
--- a/libsolidity/ast/Types.h
+++ b/libsolidity/ast/Types.h
@@ -1058,6 +1058,22 @@ public:
 	/// true iff arguments are to be padded to multiples of 32 bytes for external calls
 	bool padArguments() const { return !(m_kind == Kind::SHA3 || m_kind == Kind::SHA256 || m_kind == Kind::RIPEMD160 || m_kind == Kind::ABIEncodePacked); }
 	bool takesArbitraryParameters() const { return m_arbitraryParameters; }
+	/// true iff the function takes a single bytes parameter and it is passed on without padding.
+	/// @todo until 0.5.0, this is just a "recommendation".
+	bool takesSinglePackedBytesParameter() const
+	{
+		// @todo add the call kinds here with 0.5.0 and perhaps also log0.
+		switch (m_kind)
+		{
+		case FunctionType::Kind::SHA3:
+		case FunctionType::Kind::SHA256:
+		case FunctionType::Kind::RIPEMD160:
+			return true;
+		default:
+			return false;
+		}
+	}
+
 	bool gasSet() const { return m_gasSet; }
 	bool valueSet() const { return m_valueSet; }
 	bool bound() const { return m_bound; }
diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp
index 42f69099..1efcfde0 100644
--- a/test/libsolidity/SolidityEndToEndTest.cpp
+++ b/test/libsolidity/SolidityEndToEndTest.cpp
@@ -3959,6 +3959,51 @@ BOOST_AUTO_TEST_CASE(call_forward_bytes)
 	ABI_CHECK(callContractFunction("val()"), encodeArgs(0x80));
 }
 
+BOOST_AUTO_TEST_CASE(call_forward_bytes_length)
+{
+	char const* sourceCode = R"(
+		contract receiver {
+			uint public calledLength;
+			function() { calledLength = msg.data.length; }
+		}
+		contract sender {
+			receiver rec;
+			constructor() { rec = new receiver(); }
+			function viaCalldata() returns (uint) {
+				require(rec.call(msg.data));
+				return rec.calledLength();
+			}
+			function viaMemory() returns (uint) {
+				bytes memory x = msg.data;
+				require(rec.call(x));
+				return rec.calledLength();
+			}
+			bytes s;
+			function viaStorage() returns (uint) {
+				s = msg.data;
+				require(rec.call(s));
+				return rec.calledLength();
+			}
+		}
+	)";
+	compileAndRun(sourceCode, 0, "sender");
+
+	// No additional data, just function selector
+	ABI_CHECK(callContractFunction("viaCalldata()"), encodeArgs(4));
+	ABI_CHECK(callContractFunction("viaMemory()"), encodeArgs(0x20));
+	// Should be this with 0.5.0: encodeArgs(4));
+	ABI_CHECK(callContractFunction("viaStorage()"), encodeArgs(0x20));
+	// Should be this with 0.5.0: encodeArgs(4));
+
+	// Some additional unpadded data
+	bytes unpadded = asBytes(string("abc"));
+	ABI_CHECK(callContractFunctionNoEncoding("viaCalldata()", unpadded), encodeArgs(7));
+	ABI_CHECK(callContractFunctionNoEncoding("viaMemory()", unpadded), encodeArgs(0x20));
+	// Should be this with 0.5.0: encodeArgs(7));
+	ABI_CHECK(callContractFunctionNoEncoding("viaStorage()", unpadded), encodeArgs(0x20));
+	// Should be this with 0.5.0: encodeArgs(7));
+}
+
 BOOST_AUTO_TEST_CASE(copying_bytes_multiassign)
 {
 	char const* sourceCode = R"(
diff --git a/test/libsolidity/syntaxTests/constants/cyclic_dependency_2.sol b/test/libsolidity/syntaxTests/constants/cyclic_dependency_2.sol
index 00f9bb0f..0728c962 100644
--- a/test/libsolidity/syntaxTests/constants/cyclic_dependency_2.sol
+++ b/test/libsolidity/syntaxTests/constants/cyclic_dependency_2.sol
@@ -5,6 +5,7 @@ contract C {
     uint constant d = 2 + a;
 }
 // ----
+// Warning: (98-110): This function only accepts a single "bytes" argument. Please use "abi.encodePacked(...)" or a similar function to encode the data. The provided argument of type uint256 is not implicitly convertible to expected type bytes memory.
 // TypeError: (17-40): The value of the constant a has a cyclic dependency via c.
 // TypeError: (71-111): The value of the constant c has a cyclic dependency via d.
 // TypeError: (117-140): The value of the constant d has a cyclic dependency via a.
diff --git a/test/libsolidity/syntaxTests/constants/cyclic_dependency_4.sol b/test/libsolidity/syntaxTests/constants/cyclic_dependency_4.sol
index f01cb98e..8954e6a3 100644
--- a/test/libsolidity/syntaxTests/constants/cyclic_dependency_4.sol
+++ b/test/libsolidity/syntaxTests/constants/cyclic_dependency_4.sol
@@ -3,4 +3,6 @@ contract C {
     uint constant b = 7;
     uint constant c = 4 + uint(keccak256(d));
     uint constant d = 2 + b;
-}
\ No newline at end of file
+}
+// ----
+// Warning: (98-110): This function only accepts a single "bytes" argument. Please use "abi.encodePacked(...)" or a similar function to encode the data. The provided argument of type uint256 is not implicitly convertible to expected type bytes memory.
diff --git a/test/libsolidity/syntaxTests/deprecated_functions.sol b/test/libsolidity/syntaxTests/deprecated_functions.sol
index 57ee484a..9df2b43c 100644
--- a/test/libsolidity/syntaxTests/deprecated_functions.sol
+++ b/test/libsolidity/syntaxTests/deprecated_functions.sol
@@ -1,6 +1,6 @@
 contract test {
 	function f() pure public {
-		bytes32 x = sha3(uint8(1));
+		bytes32 x = sha3();
 		x;
 	}
 	function g() public {
@@ -8,5 +8,5 @@ contract test {
 	}
 }
 // ----
-// Warning: (58-72): "sha3" has been deprecated in favour of "keccak256"
-// Warning: (107-117): "suicide" has been deprecated in favour of "selfdestruct"
+// Warning: (58-64): "sha3" has been deprecated in favour of "keccak256"
+// Warning: (99-109): "suicide" has been deprecated in favour of "selfdestruct"
diff --git a/test/libsolidity/syntaxTests/deprecated_functions_050.sol b/test/libsolidity/syntaxTests/deprecated_functions_050.sol
index 7e36543b..92973c5f 100644
--- a/test/libsolidity/syntaxTests/deprecated_functions_050.sol
+++ b/test/libsolidity/syntaxTests/deprecated_functions_050.sol
@@ -10,4 +10,5 @@ contract test {
 }
 // ----
 // TypeError: (88-102): "sha3" has been deprecated in favour of "keccak256"
+// TypeError: (88-102): This function only accepts a single "bytes" argument. Please use "abi.encodePacked(...)" or a similar function to encode the data. The provided argument of type uint8 is not implicitly convertible to expected type bytes memory.
 // TypeError: (137-147): "suicide" has been deprecated in favour of "selfdestruct"
diff --git a/test/libsolidity/syntaxTests/specialFunctions/single_non_bytes_arg.sol b/test/libsolidity/syntaxTests/specialFunctions/single_non_bytes_arg.sol
new file mode 100644
index 00000000..3f8e3014
--- /dev/null
+++ b/test/libsolidity/syntaxTests/specialFunctions/single_non_bytes_arg.sol
@@ -0,0 +1,12 @@
+contract C {
+    function f() pure public {
+        g(keccak256(uint(2)));
+        g(sha256(uint(2)));
+        g(ripemd160(uint(2)));
+    }
+    function g(bytes32) pure internal {}
+}
+// ----
+// Warning: (54-72): This function only accepts a single "bytes" argument. Please use "abi.encodePacked(...)" or a similar function to encode the data. The provided argument of type uint256 is not implicitly convertible to expected type bytes memory.
+// Warning: (85-100): This function only accepts a single "bytes" argument. Please use "abi.encodePacked(...)" or a similar function to encode the data. The provided argument of type uint256 is not implicitly convertible to expected type bytes memory.
+// Warning: (113-131): This function only accepts a single "bytes" argument. Please use "abi.encodePacked(...)" or a similar function to encode the data. The provided argument of type uint256 is not implicitly convertible to expected type bytes memory.
diff --git a/test/libsolidity/syntaxTests/specialFunctions/types_with_unspecified_encoding_internal_functions.sol b/test/libsolidity/syntaxTests/specialFunctions/types_with_unspecified_encoding_internal_functions.sol
index c98d7a57..b94a4391 100644
--- a/test/libsolidity/syntaxTests/specialFunctions/types_with_unspecified_encoding_internal_functions.sol
+++ b/test/libsolidity/syntaxTests/specialFunctions/types_with_unspecified_encoding_internal_functions.sol
@@ -1,11 +1,11 @@
 contract C {
     function f() public pure {
-        bytes32 h = keccak256(keccak256, f, this.f.gas, block.blockhash);
+        bytes32 h = keccak256(abi.encodePacked(keccak256, f, this.f.gas, block.blockhash));
         h;
     }
 }
 // ----
-// TypeError: (74-83): This type cannot be encoded.
-// TypeError: (85-86): This type cannot be encoded.
-// TypeError: (88-98): This type cannot be encoded.
-// TypeError: (100-115): This type cannot be encoded.
+// TypeError: (91-100): This type cannot be encoded.
+// TypeError: (102-103): This type cannot be encoded.
+// TypeError: (105-115): This type cannot be encoded.
+// TypeError: (117-132): This type cannot be encoded.
diff --git a/test/libsolidity/syntaxTests/specialFunctions/types_with_unspecified_encoding_structs.sol b/test/libsolidity/syntaxTests/specialFunctions/types_with_unspecified_encoding_structs.sol
index fa910260..05f5db0b 100644
--- a/test/libsolidity/syntaxTests/specialFunctions/types_with_unspecified_encoding_structs.sol
+++ b/test/libsolidity/syntaxTests/specialFunctions/types_with_unspecified_encoding_structs.sol
@@ -9,5 +9,6 @@ contract C {
     }
 }
 // ----
+// Warning: (132-144): This function only accepts a single "bytes" argument. Please use "abi.encodePacked(...)" or a similar function to encode the data.
 // TypeError: (139-140): This type cannot be encoded.
 // TypeError: (142-143): This type cannot be encoded.
diff --git a/test/libsolidity/syntaxTests/specialFunctions/types_with_unspecified_encoding_structs_abiv2.sol b/test/libsolidity/syntaxTests/specialFunctions/types_with_unspecified_encoding_structs_abiv2.sol
index 1187ce4a..977a7d73 100644
--- a/test/libsolidity/syntaxTests/specialFunctions/types_with_unspecified_encoding_structs_abiv2.sol
+++ b/test/libsolidity/syntaxTests/specialFunctions/types_with_unspecified_encoding_structs_abiv2.sol
@@ -12,5 +12,6 @@ contract C {
 }
 // ----
 // Warning: (0-33): Experimental features are turned on. Do not use experimental features on live deployments.
+// Warning: (167-179): This function only accepts a single "bytes" argument. Please use "abi.encodePacked(...)" or a similar function to encode the data.
 // TypeError: (174-175): This type cannot be encoded.
 // TypeError: (177-178): This type cannot be encoded.
diff --git a/test/libsolidity/syntaxTests/specialFunctions/types_without_encoding_problems.sol b/test/libsolidity/syntaxTests/specialFunctions/types_without_encoding_problems.sol
index c8364548..d890e35f 100644
--- a/test/libsolidity/syntaxTests/specialFunctions/types_without_encoding_problems.sol
+++ b/test/libsolidity/syntaxTests/specialFunctions/types_without_encoding_problems.sol
@@ -2,7 +2,7 @@ contract C {
     uint[3] sarr;
     function f() view public {
         uint[3] memory arr;
-        bytes32 h = keccak256(this.f, arr, sarr);
+        bytes32 h = keccak256(abi.encodePacked(this.f, arr, sarr));
         h;
     }
 }
diff --git a/test/libsolidity/syntaxTests/tight_packing_literals.sol b/test/libsolidity/syntaxTests/tight_packing_literals.sol
index 8258a8a6..202a3202 100644
--- a/test/libsolidity/syntaxTests/tight_packing_literals.sol
+++ b/test/libsolidity/syntaxTests/tight_packing_literals.sol
@@ -18,8 +18,12 @@ contract C {
 
 // ----
 // Warning: (87-88): The type of "int_const 1" was inferred as uint8. This is probably not desired. Use an explicit type to silence this warning.
+// Warning: (77-89): This function only accepts a single "bytes" argument. Please use "abi.encodePacked(...)" or a similar function to encode the data. The provided argument of type int_const 1 is not implicitly convertible to expected type bytes memory.
 // Warning: (161-168): "sha3" has been deprecated in favour of "keccak256"
 // Warning: (166-167): The type of "int_const 1" was inferred as uint8. This is probably not desired. Use an explicit type to silence this warning.
+// Warning: (161-168): This function only accepts a single "bytes" argument. Please use "abi.encodePacked(...)" or a similar function to encode the data. The provided argument of type int_const 1 is not implicitly convertible to expected type bytes memory.
 // Warning: (247-248): The type of "int_const 1" was inferred as uint8. This is probably not desired. Use an explicit type to silence this warning.
+// Warning: (240-249): This function only accepts a single "bytes" argument. Please use "abi.encodePacked(...)" or a similar function to encode the data. The provided argument of type int_const 1 is not implicitly convertible to expected type bytes memory.
 // Warning: (331-332): The type of "int_const 1" was inferred as uint8. This is probably not desired. Use an explicit type to silence this warning.
+// Warning: (321-333): This function only accepts a single "bytes" argument. Please use "abi.encodePacked(...)" or a similar function to encode the data. The provided argument of type int_const 1 is not implicitly convertible to expected type bytes memory.
 // Warning: (420-421): The type of "int_const 1" was inferred as uint8. This is probably not desired. Use an explicit type to silence this warning.
diff --git a/test/libsolidity/syntaxTests/tight_packing_literals_050.sol b/test/libsolidity/syntaxTests/tight_packing_literals_050.sol
index ef6da75d..fcf8ec50 100644
--- a/test/libsolidity/syntaxTests/tight_packing_literals_050.sol
+++ b/test/libsolidity/syntaxTests/tight_packing_literals_050.sol
@@ -19,8 +19,12 @@ contract C {
 
 // ----
 // TypeError: (117-118): Cannot perform packed encoding for a literal. Please convert it to an explicit type first.
+// TypeError: (107-119): This function only accepts a single "bytes" argument. Please use "abi.encodePacked(...)" or a similar function to encode the data. The provided argument of type int_const 1 is not implicitly convertible to expected type bytes memory.
 // TypeError: (191-198): "sha3" has been deprecated in favour of "keccak256"
 // TypeError: (196-197): Cannot perform packed encoding for a literal. Please convert it to an explicit type first.
+// TypeError: (191-198): This function only accepts a single "bytes" argument. Please use "abi.encodePacked(...)" or a similar function to encode the data. The provided argument of type int_const 1 is not implicitly convertible to expected type bytes memory.
 // TypeError: (277-278): Cannot perform packed encoding for a literal. Please convert it to an explicit type first.
+// TypeError: (270-279): This function only accepts a single "bytes" argument. Please use "abi.encodePacked(...)" or a similar function to encode the data. The provided argument of type int_const 1 is not implicitly convertible to expected type bytes memory.
 // TypeError: (361-362): Cannot perform packed encoding for a literal. Please convert it to an explicit type first.
+// TypeError: (351-363): This function only accepts a single "bytes" argument. Please use "abi.encodePacked(...)" or a similar function to encode the data. The provided argument of type int_const 1 is not implicitly convertible to expected type bytes memory.
 // TypeError: (450-451): Cannot perform packed encoding for a literal. Please convert it to an explicit type first.
diff --git a/test/libsolidity/syntaxTests/tight_packing_literals_fine.sol b/test/libsolidity/syntaxTests/tight_packing_literals_fine.sol
index 81e69eb4..e18fbd9f 100644
--- a/test/libsolidity/syntaxTests/tight_packing_literals_fine.sol
+++ b/test/libsolidity/syntaxTests/tight_packing_literals_fine.sol
@@ -19,4 +19,8 @@ contract C {
     }
 }
 // ----
+// Warning: (77-96): This function only accepts a single "bytes" argument. Please use "abi.encodePacked(...)" or a similar function to encode the data. The provided argument of type uint8 is not implicitly convertible to expected type bytes memory.
 // Warning: (168-182): "sha3" has been deprecated in favour of "keccak256"
+// Warning: (168-182): This function only accepts a single "bytes" argument. Please use "abi.encodePacked(...)" or a similar function to encode the data. The provided argument of type uint8 is not implicitly convertible to expected type bytes memory.
+// Warning: (254-270): This function only accepts a single "bytes" argument. Please use "abi.encodePacked(...)" or a similar function to encode the data. The provided argument of type uint8 is not implicitly convertible to expected type bytes memory.
+// Warning: (342-361): This function only accepts a single "bytes" argument. Please use "abi.encodePacked(...)" or a similar function to encode the data. The provided argument of type uint8 is not implicitly convertible to expected type bytes memory.