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" ], 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(); } diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index 5bcde4411..db7f59ee5 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 == keccak256(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"(