diff --git a/Changelog.md b/Changelog.md index 6ec21a974..7c613ff1f 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,5 +1,9 @@ ### 0.6.8 (unreleased) +Important Bugfixes: + * Add missing callvalue check to the creation code of a contract that does not define a constructor but has a base that does define a constructor. + + Language Features: * Implemented ``type(X).min`` and ``type(X).max`` for every integer type ``X`` that returns the smallest and largest value representable by the type. diff --git a/docs/bugs.json b/docs/bugs.json index 56bae3aaf..c251fa114 100644 --- a/docs/bugs.json +++ b/docs/bugs.json @@ -1,4 +1,12 @@ [ + { + "name": "ImplicitConstructorCallvalueCheck", + "summary": "The creation code of a contract that does not define a constructor but has a base that does define a constructor did not revert for calls with non-zero value.", + "description": "Starting from Solidity 0.4.5 the creation code of contracts without explicit payable constructor is supposed to contain a callvalue check that results in contract creation reverting, if non-zero value is passed. However, this check was missing in case no explicit constructor was defined in a contract at all, but the contract has a base that does define a constructor. In these cases it is possible to send value in a contract creation transaction or using inline assembly without revert, even though the creation code is supposed to be non-payable.", + "introduced": "0.4.5", + "fixed": "0.6.8", + "severity": "very low" + }, { "name": "TupleAssignmentMultiStackSlotComponents", "summary": "Tuple assignments with components that occupy several stack slots, i.e. nested tuples, pointers to external functions or references to dynamically sized calldata arrays, can result in invalid values.", diff --git a/docs/bugs_by_version.json b/docs/bugs_by_version.json index 4c27e90d9..0398e4cd7 100644 --- a/docs/bugs_by_version.json +++ b/docs/bugs_by_version.json @@ -415,6 +415,7 @@ }, "0.4.10": { "bugs": [ + "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", "privateCanBeOverridden", @@ -433,6 +434,7 @@ }, "0.4.11": { "bugs": [ + "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", "privateCanBeOverridden", @@ -450,6 +452,7 @@ }, "0.4.12": { "bugs": [ + "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", "privateCanBeOverridden", @@ -466,6 +469,7 @@ }, "0.4.13": { "bugs": [ + "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", "privateCanBeOverridden", @@ -482,6 +486,7 @@ }, "0.4.14": { "bugs": [ + "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", "privateCanBeOverridden", @@ -497,6 +502,7 @@ }, "0.4.15": { "bugs": [ + "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", "privateCanBeOverridden", @@ -511,6 +517,7 @@ }, "0.4.16": { "bugs": [ + "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", "privateCanBeOverridden", @@ -527,6 +534,7 @@ }, "0.4.17": { "bugs": [ + "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", "privateCanBeOverridden", @@ -544,6 +552,7 @@ }, "0.4.18": { "bugs": [ + "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", "privateCanBeOverridden", @@ -560,6 +569,7 @@ }, "0.4.19": { "bugs": [ + "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", "privateCanBeOverridden", @@ -596,6 +606,7 @@ }, "0.4.20": { "bugs": [ + "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", "privateCanBeOverridden", @@ -613,6 +624,7 @@ }, "0.4.21": { "bugs": [ + "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", "privateCanBeOverridden", @@ -630,6 +642,7 @@ }, "0.4.22": { "bugs": [ + "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", "privateCanBeOverridden", @@ -647,6 +660,7 @@ }, "0.4.23": { "bugs": [ + "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", "privateCanBeOverridden", @@ -663,6 +677,7 @@ }, "0.4.24": { "bugs": [ + "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", "privateCanBeOverridden", @@ -679,6 +694,7 @@ }, "0.4.25": { "bugs": [ + "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", "privateCanBeOverridden", @@ -693,6 +709,7 @@ }, "0.4.26": { "bugs": [ + "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", "privateCanBeOverridden", @@ -739,6 +756,7 @@ }, "0.4.5": { "bugs": [ + "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", "privateCanBeOverridden", @@ -758,6 +776,7 @@ }, "0.4.6": { "bugs": [ + "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", "privateCanBeOverridden", @@ -776,6 +795,7 @@ }, "0.4.7": { "bugs": [ + "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", "privateCanBeOverridden", @@ -794,6 +814,7 @@ }, "0.4.8": { "bugs": [ + "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", "privateCanBeOverridden", @@ -812,6 +833,7 @@ }, "0.4.9": { "bugs": [ + "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", "privateCanBeOverridden", @@ -830,6 +852,7 @@ }, "0.5.0": { "bugs": [ + "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", "privateCanBeOverridden", @@ -844,6 +867,7 @@ }, "0.5.1": { "bugs": [ + "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", "privateCanBeOverridden", @@ -858,6 +882,7 @@ }, "0.5.10": { "bugs": [ + "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", "privateCanBeOverridden", @@ -868,6 +893,7 @@ }, "0.5.11": { "bugs": [ + "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", "privateCanBeOverridden", @@ -877,6 +903,7 @@ }, "0.5.12": { "bugs": [ + "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", "privateCanBeOverridden", @@ -886,6 +913,7 @@ }, "0.5.13": { "bugs": [ + "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", "privateCanBeOverridden", @@ -895,6 +923,7 @@ }, "0.5.14": { "bugs": [ + "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", "privateCanBeOverridden", @@ -905,6 +934,7 @@ }, "0.5.15": { "bugs": [ + "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", "privateCanBeOverridden", @@ -914,6 +944,7 @@ }, "0.5.16": { "bugs": [ + "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", "privateCanBeOverridden" @@ -922,6 +953,7 @@ }, "0.5.17": { "bugs": [ + "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow" ], @@ -929,6 +961,7 @@ }, "0.5.2": { "bugs": [ + "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", "privateCanBeOverridden", @@ -943,6 +976,7 @@ }, "0.5.3": { "bugs": [ + "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", "privateCanBeOverridden", @@ -957,6 +991,7 @@ }, "0.5.4": { "bugs": [ + "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", "privateCanBeOverridden", @@ -971,6 +1006,7 @@ }, "0.5.5": { "bugs": [ + "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", "privateCanBeOverridden", @@ -987,6 +1023,7 @@ }, "0.5.6": { "bugs": [ + "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", "privateCanBeOverridden", @@ -1003,6 +1040,7 @@ }, "0.5.7": { "bugs": [ + "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", "privateCanBeOverridden", @@ -1017,6 +1055,7 @@ }, "0.5.8": { "bugs": [ + "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", "privateCanBeOverridden", @@ -1030,6 +1069,7 @@ }, "0.5.9": { "bugs": [ + "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", "privateCanBeOverridden", @@ -1042,6 +1082,7 @@ }, "0.6.0": { "bugs": [ + "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow", "YulOptimizerRedundantAssignmentBreakContinue" @@ -1050,6 +1091,7 @@ }, "0.6.1": { "bugs": [ + "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow" ], @@ -1057,6 +1099,7 @@ }, "0.6.2": { "bugs": [ + "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow" ], @@ -1064,6 +1107,7 @@ }, "0.6.3": { "bugs": [ + "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow" ], @@ -1071,6 +1115,7 @@ }, "0.6.4": { "bugs": [ + "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents", "MemoryArrayCreationOverflow" ], @@ -1078,16 +1123,21 @@ }, "0.6.5": { "bugs": [ + "ImplicitConstructorCallvalueCheck", "TupleAssignmentMultiStackSlotComponents" ], "released": "2020-04-06" }, "0.6.6": { - "bugs": [], + "bugs": [ + "ImplicitConstructorCallvalueCheck" + ], "released": "2020-04-09" }, "0.6.7": { - "bugs": [], + "bugs": [ + "ImplicitConstructorCallvalueCheck" + ], "released": "2020-05-04" } } \ No newline at end of file diff --git a/libsolidity/codegen/ContractCompiler.cpp b/libsolidity/codegen/ContractCompiler.cpp index 678921824..269d9a5bf 100644 --- a/libsolidity/codegen/ContractCompiler.cpp +++ b/libsolidity/codegen/ContractCompiler.cpp @@ -157,10 +157,13 @@ void ContractCompiler::appendInitAndConstructorCode(ContractDefinition const& _c if (FunctionDefinition const* constructor = _contract.constructor()) appendConstructor(*constructor); - else if (auto c = _contract.nextConstructor(m_context.mostDerivedContract())) - appendBaseConstructor(*c); else + { + // Implicit constructors are always non-payable. appendCallValueCheck(); + if (auto c = _contract.nextConstructor(m_context.mostDerivedContract())) + appendBaseConstructor(*c); + } } size_t ContractCompiler::packIntoContractCreator(ContractDefinition const& _contract) diff --git a/test/libsolidity/semanticTests/constructor/callvalue_check.sol b/test/libsolidity/semanticTests/constructor/callvalue_check.sol new file mode 100644 index 000000000..7a327e695 --- /dev/null +++ b/test/libsolidity/semanticTests/constructor/callvalue_check.sol @@ -0,0 +1,40 @@ +contract A1 { constructor() public {} } +contract B1 is A1 {} + +contract A2 { constructor() public payable {} } +contract B2 is A2 {} + +contract B3 {} + +contract B4 { constructor() public {} } + +contract C { + function createWithValue(bytes memory c, uint256 value) public payable returns (bool) { + uint256 y = 0; + assembly { y := create(value, add(c, 0x20), mload(c)) } + return y != 0; + } + function f(uint256 value) public payable returns (bool) { + return createWithValue(type(B1).creationCode, value); + } + function g(uint256 value) public payable returns (bool) { + return createWithValue(type(B2).creationCode, value); + } + function h(uint256 value) public payable returns (bool) { + return createWithValue(type(B3).creationCode, value); + } + function i(uint256 value) public payable returns (bool) { + return createWithValue(type(B4).creationCode, value); + } +} +// ==== +// EVMVersion: >homestead +// ---- +// f(uint256), 2000 ether: 0 -> true +// f(uint256), 2000 ether: 100 -> false +// g(uint256), 2000 ether: 0 -> true +// g(uint256), 2000 ether: 100 -> false +// h(uint256), 2000 ether: 0 -> true +// h(uint256), 2000 ether: 100 -> false +// i(uint256), 2000 ether: 0 -> true +// i(uint256), 2000 ether: 100 -> false diff --git a/test/libsolidity/semanticTests/constructor/no_callvalue_check.sol b/test/libsolidity/semanticTests/constructor/no_callvalue_check.sol new file mode 100644 index 000000000..9744aa3ce --- /dev/null +++ b/test/libsolidity/semanticTests/constructor/no_callvalue_check.sol @@ -0,0 +1,21 @@ +contract A1 {} +contract B1 is A1 { constructor() public payable {} } + +contract A2 { constructor() public {} } +contract B2 is A2 { constructor() public payable {} } + +contract B3 { constructor() public payable {} } + +contract C { + function f() public payable returns (bool) { + // Make sure none of these revert. + new B1{value: 10}(); + new B2{value: 10}(); + new B3{value: 10}(); + return true; + } +} +// ==== +// compileViaYul: also +// ---- +// f(), 2000 ether -> true diff --git a/test/libsolidity/syntaxTests/constructor/nonpayable_new.sol b/test/libsolidity/syntaxTests/constructor/nonpayable_new.sol new file mode 100644 index 000000000..c89508743 --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/nonpayable_new.sol @@ -0,0 +1,23 @@ +contract A1 { constructor() public {} } +contract B1 is A1 {} + +contract A2 { constructor() public payable {} } +contract B2 is A2 {} + +contract B3 {} + +contract B4 { constructor() public {} } + +contract C { + function f() public payable { + new B1{value: 10}(); + new B2{value: 10}(); + new B3{value: 10}(); + new B4{value: 10}(); + } +} +// ---- +// TypeError: (235-252): Cannot set option "value", since the constructor of contract B1 is not payable. +// TypeError: (258-275): Cannot set option "value", since the constructor of contract B2 is not payable. +// TypeError: (281-298): Cannot set option "value", since the constructor of contract B3 is not payable. +// TypeError: (304-321): Cannot set option "value", since the constructor of contract B4 is not payable. diff --git a/test/libsolidity/syntaxTests/constructor/payable_new.sol b/test/libsolidity/syntaxTests/constructor/payable_new.sol new file mode 100644 index 000000000..fb8e9ef53 --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/payable_new.sol @@ -0,0 +1,16 @@ +contract A1 {} +contract B1 is A1 { constructor() public payable {} } + +contract A2 { constructor() public {} } +contract B2 is A2 { constructor() public payable {} } + +contract B3 { constructor() public payable {} } + +contract C { + function f() public payable { + new B1{value: 10}(); + new B2{value: 10}(); + new B3{value: 10}(); + } +} +// ----