From 368a8a62c1c2c34cdcd1edb3b1ce1d5a2a3a2870 Mon Sep 17 00:00:00 2001 From: chriseth Date: Fri, 28 Jul 2017 10:06:33 +0200 Subject: [PATCH 1/4] Test case for invalid ecrecover call. --- test/libsolidity/SolidityEndToEndTest.cpp | 47 +++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index 5bcde4411..057ec94bc 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -8251,6 +8251,53 @@ BOOST_AUTO_TEST_CASE(failing_ecrecover_invalid_input) BOOST_CHECK(callContractFunction("f()") == encodeArgs(u256(0))); } +BOOST_AUTO_TEST_CASE(failing_ecrecover_invalid_input_proper) +{ + char const* sourceCode = R"( + contract C { + function f() returns (address) { + return recover( + 0x77e5189111eb6557e8a637b27ef8fbb15bc61d61c2f00cc48878f3a296e5e0ca, + 0, // invalid v value + 0x6944c77849b18048f6abe0db8084b0d0d0689cdddb53d2671c36967b58691ad4, + 0xef4f06ba4f78319baafd0424365777241af4dfd3da840471b4b4b087b7750d0d, + 0xca35b7d915458ef540ade6068dfe2f44e8fa733c, + 0xca35b7d915458ef540ade6068dfe2f44e8fa733c + ); + } + function recover(bytes32 hash, uint8 v, bytes32 r, bytes32 s, uint blockExpired, bytes32 salt) + returns (address) + { + require(hash == sha3(blockExpired, salt)); + return ecrecover(hash, v, r, s); + } + } + )"; + compileAndRun(sourceCode, 0, "C"); + BOOST_CHECK(callContractFunction("f()") == encodeArgs(u256(0))); +} + +BOOST_AUTO_TEST_CASE(failing_ecrecover_invalid_input_asm) +{ + char const* sourceCode = R"( + contract C { + function f() returns (address) { + assembly { + mstore(mload(0x40), 0xca35b7d915458ef540ade6068dfe2f44e8fa733c) + } + return ecrecover( + 0x77e5189111eb6557e8a637b27ef8fbb15bc61d61c2f00cc48878f3a296e5e0ca, + 0, // invalid v value + 0x6944c77849b18048f6abe0db8084b0d0d0689cdddb53d2671c36967b58691ad4, + 0xef4f06ba4f78319baafd0424365777241af4dfd3da840471b4b4b087b7750d0d + ); + } + } + )"; + compileAndRun(sourceCode, 0, "C"); + BOOST_CHECK(callContractFunction("f()") == encodeArgs(u256(0))); +} + BOOST_AUTO_TEST_CASE(calling_nonexisting_contract_throws) { char const* sourceCode = R"( From b74118ec57b2316c843961fc3e1dbb2e5eef35dd Mon Sep 17 00:00:00 2001 From: chriseth Date: Fri, 28 Jul 2017 10:06:49 +0200 Subject: [PATCH 2/4] Fix for invalid clearing of memory in ecrecover. --- libsolidity/codegen/ExpressionCompiler.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libsolidity/codegen/ExpressionCompiler.cpp b/libsolidity/codegen/ExpressionCompiler.cpp index a35008bf2..02cc62be0 100644 --- a/libsolidity/codegen/ExpressionCompiler.cpp +++ b/libsolidity/codegen/ExpressionCompiler.cpp @@ -1622,7 +1622,7 @@ void ExpressionCompiler::appendExternalFunctionCall( // zero bytes (which we cannot detect). solAssert(0 < retSize && retSize <= 32, ""); utils().fetchFreeMemoryPointer(); - m_context << Instruction::DUP1 << u256(0) << Instruction::MSTORE; + m_context << u256(0) << Instruction::DUP2 << Instruction::MSTORE; m_context << u256(32) << Instruction::ADD; utils().storeFreeMemoryPointer(); } From 2cdb5c9e83add0184e51b6cdc03d19f560f4a574 Mon Sep 17 00:00:00 2001 From: chriseth Date: Fri, 28 Jul 2017 12:05:43 +0200 Subject: [PATCH 3/4] Changelog and bug list entry. --- Changelog.md | 1 + docs/bugs.json | 9 ++++++++- docs/bugs_by_version.json | 38 ++++++++++++++++++++++++++++++++++++-- 3 files changed, 45 insertions(+), 3 deletions(-) diff --git a/Changelog.md b/Changelog.md index cedb58591..6bf998b5d 100644 --- a/Changelog.md +++ b/Changelog.md @@ -13,6 +13,7 @@ Features: * Type checker: Warn when existing symbols, including builtins, are overwritten. Bugfixes: + * Code Generator: Properly clear return memory area for ecrecover. * Type Checker: Fix crash for some assignment to non-lvalue. * Type Checker: Fix invalid "specify storage keyword" warning for reference members of structs. * Type Checker: Mark modifiers as internal. diff --git a/docs/bugs.json b/docs/bugs.json index a0c0e7c43..4fd73492d 100644 --- a/docs/bugs.json +++ b/docs/bugs.json @@ -1,4 +1,11 @@ [ + { + "name": "ECRecoverMalformedInput", + "summary": "The ecrecover() builtin can return garbage for malformed input.", + "description": "The ecrecover precompile does not properly signal failure for malformed input (especially in the 'v' argument) and thus the Solidity function can return data that was previously present in the return area in memory.", + "fixed": "0.4.14", + "severity": "medium" + }, { "name": "SkipEmptyStringLiteral", "summary": "If \"\" is used in a function call, the following function arguments will not be correctly passed to the function.", @@ -107,4 +114,4 @@ "severity": "high", "fixed": "0.3.0" } -] \ No newline at end of file +] diff --git a/docs/bugs_by_version.json b/docs/bugs_by_version.json index d6802eec3..e67a08453 100644 --- a/docs/bugs_by_version.json +++ b/docs/bugs_by_version.json @@ -1,6 +1,7 @@ { "0.1.0": { "bugs": [ + "ECRecoverMalformedInput", "SkipEmptyStringLiteral", "ConstantOptimizerSubtraction", "IdentityPrecompileReturnIgnored", @@ -16,6 +17,7 @@ }, "0.1.1": { "bugs": [ + "ECRecoverMalformedInput", "SkipEmptyStringLiteral", "ConstantOptimizerSubtraction", "IdentityPrecompileReturnIgnored", @@ -31,6 +33,7 @@ }, "0.1.2": { "bugs": [ + "ECRecoverMalformedInput", "SkipEmptyStringLiteral", "ConstantOptimizerSubtraction", "IdentityPrecompileReturnIgnored", @@ -46,6 +49,7 @@ }, "0.1.3": { "bugs": [ + "ECRecoverMalformedInput", "SkipEmptyStringLiteral", "ConstantOptimizerSubtraction", "IdentityPrecompileReturnIgnored", @@ -61,6 +65,7 @@ }, "0.1.4": { "bugs": [ + "ECRecoverMalformedInput", "SkipEmptyStringLiteral", "ConstantOptimizerSubtraction", "IdentityPrecompileReturnIgnored", @@ -76,6 +81,7 @@ }, "0.1.5": { "bugs": [ + "ECRecoverMalformedInput", "SkipEmptyStringLiteral", "ConstantOptimizerSubtraction", "IdentityPrecompileReturnIgnored", @@ -91,6 +97,7 @@ }, "0.1.6": { "bugs": [ + "ECRecoverMalformedInput", "SkipEmptyStringLiteral", "ConstantOptimizerSubtraction", "IdentityPrecompileReturnIgnored", @@ -107,6 +114,7 @@ }, "0.1.7": { "bugs": [ + "ECRecoverMalformedInput", "SkipEmptyStringLiteral", "ConstantOptimizerSubtraction", "IdentityPrecompileReturnIgnored", @@ -123,6 +131,7 @@ }, "0.2.0": { "bugs": [ + "ECRecoverMalformedInput", "SkipEmptyStringLiteral", "ConstantOptimizerSubtraction", "IdentityPrecompileReturnIgnored", @@ -139,6 +148,7 @@ }, "0.2.1": { "bugs": [ + "ECRecoverMalformedInput", "SkipEmptyStringLiteral", "ConstantOptimizerSubtraction", "IdentityPrecompileReturnIgnored", @@ -155,6 +165,7 @@ }, "0.2.2": { "bugs": [ + "ECRecoverMalformedInput", "SkipEmptyStringLiteral", "ConstantOptimizerSubtraction", "IdentityPrecompileReturnIgnored", @@ -171,6 +182,7 @@ }, "0.3.0": { "bugs": [ + "ECRecoverMalformedInput", "SkipEmptyStringLiteral", "ConstantOptimizerSubtraction", "IdentityPrecompileReturnIgnored", @@ -186,6 +198,7 @@ }, "0.3.1": { "bugs": [ + "ECRecoverMalformedInput", "SkipEmptyStringLiteral", "ConstantOptimizerSubtraction", "IdentityPrecompileReturnIgnored", @@ -200,6 +213,7 @@ }, "0.3.2": { "bugs": [ + "ECRecoverMalformedInput", "SkipEmptyStringLiteral", "ConstantOptimizerSubtraction", "IdentityPrecompileReturnIgnored", @@ -214,6 +228,7 @@ }, "0.3.3": { "bugs": [ + "ECRecoverMalformedInput", "SkipEmptyStringLiteral", "ConstantOptimizerSubtraction", "IdentityPrecompileReturnIgnored", @@ -227,6 +242,7 @@ }, "0.3.4": { "bugs": [ + "ECRecoverMalformedInput", "SkipEmptyStringLiteral", "ConstantOptimizerSubtraction", "IdentityPrecompileReturnIgnored", @@ -240,6 +256,7 @@ }, "0.3.5": { "bugs": [ + "ECRecoverMalformedInput", "SkipEmptyStringLiteral", "ConstantOptimizerSubtraction", "IdentityPrecompileReturnIgnored", @@ -253,6 +270,7 @@ }, "0.3.6": { "bugs": [ + "ECRecoverMalformedInput", "SkipEmptyStringLiteral", "ConstantOptimizerSubtraction", "IdentityPrecompileReturnIgnored", @@ -264,6 +282,7 @@ }, "0.4.0": { "bugs": [ + "ECRecoverMalformedInput", "SkipEmptyStringLiteral", "ConstantOptimizerSubtraction", "IdentityPrecompileReturnIgnored", @@ -275,6 +294,7 @@ }, "0.4.1": { "bugs": [ + "ECRecoverMalformedInput", "SkipEmptyStringLiteral", "ConstantOptimizerSubtraction", "IdentityPrecompileReturnIgnored", @@ -286,6 +306,7 @@ }, "0.4.10": { "bugs": [ + "ECRecoverMalformedInput", "SkipEmptyStringLiteral", "ConstantOptimizerSubtraction" ], @@ -293,20 +314,26 @@ }, "0.4.11": { "bugs": [ + "ECRecoverMalformedInput", "SkipEmptyStringLiteral" ], "released": "2017-05-03" }, "0.4.12": { - "bugs": [], + "bugs": [ + "ECRecoverMalformedInput" + ], "released": "2017-07-03" }, "0.4.13": { - "bugs": [], + "bugs": [ + "ECRecoverMalformedInput" + ], "released": "2017-07-06" }, "0.4.2": { "bugs": [ + "ECRecoverMalformedInput", "SkipEmptyStringLiteral", "ConstantOptimizerSubtraction", "IdentityPrecompileReturnIgnored", @@ -317,6 +344,7 @@ }, "0.4.3": { "bugs": [ + "ECRecoverMalformedInput", "SkipEmptyStringLiteral", "ConstantOptimizerSubtraction", "IdentityPrecompileReturnIgnored", @@ -326,6 +354,7 @@ }, "0.4.4": { "bugs": [ + "ECRecoverMalformedInput", "SkipEmptyStringLiteral", "ConstantOptimizerSubtraction", "IdentityPrecompileReturnIgnored" @@ -334,6 +363,7 @@ }, "0.4.5": { "bugs": [ + "ECRecoverMalformedInput", "SkipEmptyStringLiteral", "ConstantOptimizerSubtraction", "IdentityPrecompileReturnIgnored", @@ -343,6 +373,7 @@ }, "0.4.6": { "bugs": [ + "ECRecoverMalformedInput", "SkipEmptyStringLiteral", "ConstantOptimizerSubtraction", "IdentityPrecompileReturnIgnored" @@ -351,6 +382,7 @@ }, "0.4.7": { "bugs": [ + "ECRecoverMalformedInput", "SkipEmptyStringLiteral", "ConstantOptimizerSubtraction" ], @@ -358,6 +390,7 @@ }, "0.4.8": { "bugs": [ + "ECRecoverMalformedInput", "SkipEmptyStringLiteral", "ConstantOptimizerSubtraction" ], @@ -365,6 +398,7 @@ }, "0.4.9": { "bugs": [ + "ECRecoverMalformedInput", "SkipEmptyStringLiteral", "ConstantOptimizerSubtraction" ], From 6dba8cf0f47a41bece23d11de9da82bb19a572e1 Mon Sep 17 00:00:00 2001 From: chriseth Date: Fri, 28 Jul 2017 16:57:34 +0200 Subject: [PATCH 4/4] Use keccak256 instead of sha3 --- test/libsolidity/SolidityEndToEndTest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index 057ec94bc..db7f59ee5 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -8268,7 +8268,7 @@ BOOST_AUTO_TEST_CASE(failing_ecrecover_invalid_input_proper) function recover(bytes32 hash, uint8 v, bytes32 r, bytes32 s, uint blockExpired, bytes32 salt) returns (address) { - require(hash == sha3(blockExpired, salt)); + require(hash == keccak256(blockExpired, salt)); return ecrecover(hash, v, r, s); } }