From 768ea5b0bee99aee6d74600ec37b2cdea906c77f Mon Sep 17 00:00:00 2001 From: Leonardo Alt Date: Wed, 15 Aug 2018 12:30:29 +0200 Subject: [PATCH 1/5] Disallow indexed reference types in events when using ABIEncoderV2 --- Changelog.md | 1 + libsolidity/analysis/TypeChecker.cpp | 10 ++++++++++ .../syntaxTests/events/event_array_indexed_v2.sol | 7 +++++++ test/libsolidity/syntaxTests/events/event_array_v2.sol | 6 ++++++ .../events/event_nested_array_indexed_v2.sol | 7 +++++++ .../syntaxTests/events/event_nested_array_v2.sol | 6 ++++++ .../syntaxTests/events/event_struct_indexed_v2.sol | 8 ++++++++ .../libsolidity/syntaxTests/events/event_struct_v2.sol | 7 +++++++ 8 files changed, 52 insertions(+) create mode 100644 test/libsolidity/syntaxTests/events/event_array_indexed_v2.sol create mode 100644 test/libsolidity/syntaxTests/events/event_array_v2.sol create mode 100644 test/libsolidity/syntaxTests/events/event_nested_array_indexed_v2.sol create mode 100644 test/libsolidity/syntaxTests/events/event_nested_array_v2.sol create mode 100644 test/libsolidity/syntaxTests/events/event_struct_indexed_v2.sol create mode 100644 test/libsolidity/syntaxTests/events/event_struct_v2.sol diff --git a/Changelog.md b/Changelog.md index 26f7d5e20..6397b23bc 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,6 +1,7 @@ ### 0.4.25 (unreleased) Bugfixes: + * Type Checker: Report error when using indexed structs in events with experimental ABIEncoderV2. This used to log wrong values. * Type Checker: Report error when using structs in events without experimental ABIEncoderV2. This used to crash or log the wrong values. ### 0.4.24 (2018-05-16) diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index f69808726..f786a83d5 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -869,7 +869,17 @@ bool TypeChecker::visit(EventDefinition const& _eventDef) for (ASTPointer const& var: _eventDef.parameters()) { if (var->isIndexed()) + { numIndexed++; + if ( + _eventDef.sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::ABIEncoderV2) + && dynamic_cast(type(*var).get()) + ) + m_errorReporter.typeError( + var->location(), + "Reference types cannot be indexed." + ); + } if (!type(*var)->canLiveOutsideStorage()) m_errorReporter.typeError(var->location(), "Type is required to live outside storage."); if (!type(*var)->interfaceType(false)) diff --git a/test/libsolidity/syntaxTests/events/event_array_indexed_v2.sol b/test/libsolidity/syntaxTests/events/event_array_indexed_v2.sol new file mode 100644 index 000000000..6b126db4a --- /dev/null +++ b/test/libsolidity/syntaxTests/events/event_array_indexed_v2.sol @@ -0,0 +1,7 @@ +pragma experimental ABIEncoderV2; +contract c { + event E(uint[] indexed); +} +// ---- +// Warning: (0-33): Experimental features are turned on. Do not use experimental features on live deployments. +// TypeError: (59-65): Reference types cannot be indexed. diff --git a/test/libsolidity/syntaxTests/events/event_array_v2.sol b/test/libsolidity/syntaxTests/events/event_array_v2.sol new file mode 100644 index 000000000..9ccd9fc90 --- /dev/null +++ b/test/libsolidity/syntaxTests/events/event_array_v2.sol @@ -0,0 +1,6 @@ +pragma experimental ABIEncoderV2; +contract c { + event E(uint[]); +} +// ---- +// Warning: (0-33): Experimental features are turned on. Do not use experimental features on live deployments. diff --git a/test/libsolidity/syntaxTests/events/event_nested_array_indexed_v2.sol b/test/libsolidity/syntaxTests/events/event_nested_array_indexed_v2.sol new file mode 100644 index 000000000..5c556125e --- /dev/null +++ b/test/libsolidity/syntaxTests/events/event_nested_array_indexed_v2.sol @@ -0,0 +1,7 @@ +pragma experimental ABIEncoderV2; +contract c { + event E(uint[][] indexed); +} +// ---- +// Warning: (0-33): Experimental features are turned on. Do not use experimental features on live deployments. +// TypeError: (59-67): Reference types cannot be indexed. diff --git a/test/libsolidity/syntaxTests/events/event_nested_array_v2.sol b/test/libsolidity/syntaxTests/events/event_nested_array_v2.sol new file mode 100644 index 000000000..efc7439ec --- /dev/null +++ b/test/libsolidity/syntaxTests/events/event_nested_array_v2.sol @@ -0,0 +1,6 @@ +pragma experimental ABIEncoderV2; +contract c { + event E(uint[][]); +} +// ---- +// Warning: (0-33): Experimental features are turned on. Do not use experimental features on live deployments. diff --git a/test/libsolidity/syntaxTests/events/event_struct_indexed_v2.sol b/test/libsolidity/syntaxTests/events/event_struct_indexed_v2.sol new file mode 100644 index 000000000..8d2d4f8c4 --- /dev/null +++ b/test/libsolidity/syntaxTests/events/event_struct_indexed_v2.sol @@ -0,0 +1,8 @@ +pragma experimental ABIEncoderV2; +contract c { + struct S { uint a ; } + event E(S indexed); +} +// ---- +// Warning: (0-33): Experimental features are turned on. Do not use experimental features on live deployments. +// TypeError: (85-86): Reference types cannot be indexed. diff --git a/test/libsolidity/syntaxTests/events/event_struct_v2.sol b/test/libsolidity/syntaxTests/events/event_struct_v2.sol new file mode 100644 index 000000000..97ca61b61 --- /dev/null +++ b/test/libsolidity/syntaxTests/events/event_struct_v2.sol @@ -0,0 +1,7 @@ +pragma experimental ABIEncoderV2; +contract c { + struct S { uint a ; } + event E(S); +} +// ---- +// Warning: (0-33): Experimental features are turned on. Do not use experimental features on live deployments. From bc6366fd5231b1542e50acb9519535903134a44a Mon Sep 17 00:00:00 2001 From: Leonardo Alt Date: Wed, 15 Aug 2018 16:54:13 +0200 Subject: [PATCH 2/5] Add end to end tests --- test/libsolidity/SolidityEndToEndTest.cpp | 203 ++++++++++++++++++++++ 1 file changed, 203 insertions(+) diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index 1efcfde0b..a4b597616 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -3516,6 +3516,209 @@ BOOST_AUTO_TEST_CASE(event_really_really_lots_of_data_from_storage) BOOST_CHECK_EQUAL(m_logs[0].topics[0], dev::keccak256(string("Deposit(uint256,bytes,uint256)"))); } +BOOST_AUTO_TEST_CASE(event_struct_memory_v2) +{ + char const* sourceCode = R"( + pragma experimental ABIEncoderV2; + contract C { + struct S { uint a; } + event E(S); + function createEvent(uint x) public { + emit E(S(x)); + } + } + )"; + compileAndRun(sourceCode); + u256 x(42); + callContractFunction("createEvent(uint256)", x); + BOOST_REQUIRE_EQUAL(m_logs.size(), 1); + BOOST_CHECK_EQUAL(m_logs[0].address, m_contractAddress); + BOOST_CHECK(m_logs[0].data == encodeArgs(x)); + BOOST_REQUIRE_EQUAL(m_logs[0].topics.size(), 1); + BOOST_CHECK_EQUAL(m_logs[0].topics[0], dev::keccak256(string("E((uint256))"))); +} + +BOOST_AUTO_TEST_CASE(event_struct_storage_v2) +{ + char const* sourceCode = R"( + pragma experimental ABIEncoderV2; + contract C { + struct S { uint a; } + event E(S); + S s; + function createEvent(uint x) public { + s.a = x; + emit E(s); + } + } + )"; + compileAndRun(sourceCode); + u256 x(42); + callContractFunction("createEvent(uint256)", x); + BOOST_REQUIRE_EQUAL(m_logs.size(), 1); + BOOST_CHECK_EQUAL(m_logs[0].address, m_contractAddress); + BOOST_CHECK(m_logs[0].data == encodeArgs(x)); + BOOST_REQUIRE_EQUAL(m_logs[0].topics.size(), 1); + BOOST_CHECK_EQUAL(m_logs[0].topics[0], dev::keccak256(string("E((uint256))"))); +} + +BOOST_AUTO_TEST_CASE(event_dynamic_array_memory) +{ + char const* sourceCode = R"( + contract C { + event E(uint[]); + function createEvent(uint x) public { + uint[] memory arr = new uint[](3); + arr[0] = x; + arr[1] = x + 1; + arr[2] = x + 2; + emit E(arr); + } + } + )"; + compileAndRun(sourceCode); + u256 x(42); + callContractFunction("createEvent(uint256)", x); + BOOST_REQUIRE_EQUAL(m_logs.size(), 1); + BOOST_CHECK_EQUAL(m_logs[0].address, m_contractAddress); + BOOST_CHECK(m_logs[0].data == encodeArgs(0x20, 3, x, x + 1, x + 2)); + BOOST_REQUIRE_EQUAL(m_logs[0].topics.size(), 1); + BOOST_CHECK_EQUAL(m_logs[0].topics[0], dev::keccak256(string("E(uint256[])"))); +} + +BOOST_AUTO_TEST_CASE(event_dynamic_array_memory_v2) +{ + char const* sourceCode = R"( + pragma experimental ABIEncoderV2; + contract C { + event E(uint[]); + function createEvent(uint x) public { + uint[] memory arr = new uint[](3); + arr[0] = x; + arr[1] = x + 1; + arr[2] = x + 2; + emit E(arr); + } + } + )"; + compileAndRun(sourceCode); + u256 x(42); + callContractFunction("createEvent(uint256)", x); + BOOST_REQUIRE_EQUAL(m_logs.size(), 1); + BOOST_CHECK_EQUAL(m_logs[0].address, m_contractAddress); + BOOST_CHECK(m_logs[0].data == encodeArgs(0x20, 3, x, x + 1, x + 2)); + BOOST_REQUIRE_EQUAL(m_logs[0].topics.size(), 1); + BOOST_CHECK_EQUAL(m_logs[0].topics[0], dev::keccak256(string("E(uint256[])"))); +} + +BOOST_AUTO_TEST_CASE(event_dynamic_nested_array_memory_v2) +{ + char const* sourceCode = R"( + pragma experimental ABIEncoderV2; + contract C { + event E(uint[][]); + function createEvent(uint x) public { + uint[][] memory arr = new uint[][](2); + arr[0] = new uint[](2); + arr[1] = new uint[](2); + arr[0][0] = x; + arr[0][1] = x + 1; + arr[1][0] = x + 2; + arr[1][1] = x + 3; + emit E(arr); + } + } + )"; + compileAndRun(sourceCode); + u256 x(42); + callContractFunction("createEvent(uint256)", x); + BOOST_REQUIRE_EQUAL(m_logs.size(), 1); + BOOST_CHECK_EQUAL(m_logs[0].address, m_contractAddress); + BOOST_CHECK(m_logs[0].data == encodeArgs(0x20, 2, 0x40, 0xa0, 2, x, x + 1, 2, x + 2, x + 3)); + BOOST_REQUIRE_EQUAL(m_logs[0].topics.size(), 1); + BOOST_CHECK_EQUAL(m_logs[0].topics[0], dev::keccak256(string("E(uint256[][])"))); +} + +BOOST_AUTO_TEST_CASE(event_dynamic_array_storage) +{ + char const* sourceCode = R"( + contract C { + event E(uint[]); + uint[] arr; + function createEvent(uint x) public { + arr.length = 3; + arr[0] = x; + arr[1] = x + 1; + arr[2] = x + 2; + emit E(arr); + } + } + )"; + compileAndRun(sourceCode); + u256 x(42); + callContractFunction("createEvent(uint256)", x); + BOOST_REQUIRE_EQUAL(m_logs.size(), 1); + BOOST_CHECK_EQUAL(m_logs[0].address, m_contractAddress); + BOOST_CHECK(m_logs[0].data == encodeArgs(0x20, 3, x, x + 1, x + 2)); + BOOST_REQUIRE_EQUAL(m_logs[0].topics.size(), 1); + BOOST_CHECK_EQUAL(m_logs[0].topics[0], dev::keccak256(string("E(uint256[])"))); +} + +BOOST_AUTO_TEST_CASE(event_dynamic_array_storage_v2) +{ + char const* sourceCode = R"( + pragma experimental ABIEncoderV2; + contract C { + event E(uint[]); + uint[] arr; + function createEvent(uint x) public { + arr.length = 3; + arr[0] = x; + arr[1] = x + 1; + arr[2] = x + 2; + emit E(arr); + } + } + )"; + compileAndRun(sourceCode); + u256 x(42); + callContractFunction("createEvent(uint256)", x); + BOOST_REQUIRE_EQUAL(m_logs.size(), 1); + BOOST_CHECK_EQUAL(m_logs[0].address, m_contractAddress); + BOOST_CHECK(m_logs[0].data == encodeArgs(0x20, 3, x, x + 1, x + 2)); + BOOST_REQUIRE_EQUAL(m_logs[0].topics.size(), 1); + BOOST_CHECK_EQUAL(m_logs[0].topics[0], dev::keccak256(string("E(uint256[])"))); +} + +BOOST_AUTO_TEST_CASE(event_dynamic_nested_array_storage_v2) +{ + char const* sourceCode = R"( + pragma experimental ABIEncoderV2; + contract C { + event E(uint[][]); + uint[][] arr; + function createEvent(uint x) public { + arr.length = 2; + arr[0].length = 2; + arr[1].length = 2; + arr[0][0] = x; + arr[0][1] = x + 1; + arr[1][0] = x + 2; + arr[1][1] = x + 3; + emit E(arr); + } + } + )"; + compileAndRun(sourceCode); + u256 x(42); + callContractFunction("createEvent(uint256)", x); + BOOST_REQUIRE_EQUAL(m_logs.size(), 1); + BOOST_CHECK_EQUAL(m_logs[0].address, m_contractAddress); + BOOST_CHECK(m_logs[0].data == encodeArgs(0x20, 2, 0x40, 0xa0, 2, x, x + 1, 2, x + 2, x + 3)); + BOOST_REQUIRE_EQUAL(m_logs[0].topics.size(), 1); + BOOST_CHECK_EQUAL(m_logs[0].topics[0], dev::keccak256(string("E(uint256[][])"))); +} + BOOST_AUTO_TEST_CASE(event_indexed_string) { char const* sourceCode = R"( From 13c8bb1c8920b6ea507688e24adb68e3bf0aed9f Mon Sep 17 00:00:00 2001 From: Leonardo Alt Date: Wed, 15 Aug 2018 16:58:41 +0200 Subject: [PATCH 3/5] Review suggestions --- libsolidity/analysis/TypeChecker.cpp | 6 +++--- .../syntaxTests/events/event_array_indexed_v2.sol | 2 +- .../syntaxTests/events/event_nested_array_indexed_v2.sol | 2 +- .../syntaxTests/events/event_struct_indexed_v2.sol | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index f786a83d5..2062458ef 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -872,12 +872,12 @@ bool TypeChecker::visit(EventDefinition const& _eventDef) { numIndexed++; if ( - _eventDef.sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::ABIEncoderV2) - && dynamic_cast(type(*var).get()) + _eventDef.sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::ABIEncoderV2) && + dynamic_cast(type(*var).get()) ) m_errorReporter.typeError( var->location(), - "Reference types cannot be indexed." + "Indexed reference types cannot yet be used with ABIEncoderV2." ); } if (!type(*var)->canLiveOutsideStorage()) diff --git a/test/libsolidity/syntaxTests/events/event_array_indexed_v2.sol b/test/libsolidity/syntaxTests/events/event_array_indexed_v2.sol index 6b126db4a..aaf6028a8 100644 --- a/test/libsolidity/syntaxTests/events/event_array_indexed_v2.sol +++ b/test/libsolidity/syntaxTests/events/event_array_indexed_v2.sol @@ -4,4 +4,4 @@ contract c { } // ---- // Warning: (0-33): Experimental features are turned on. Do not use experimental features on live deployments. -// TypeError: (59-65): Reference types cannot be indexed. +// TypeError: (59-65): Indexed reference types cannot yet be used with ABIEncoderV2. diff --git a/test/libsolidity/syntaxTests/events/event_nested_array_indexed_v2.sol b/test/libsolidity/syntaxTests/events/event_nested_array_indexed_v2.sol index 5c556125e..ffae5b9cc 100644 --- a/test/libsolidity/syntaxTests/events/event_nested_array_indexed_v2.sol +++ b/test/libsolidity/syntaxTests/events/event_nested_array_indexed_v2.sol @@ -4,4 +4,4 @@ contract c { } // ---- // Warning: (0-33): Experimental features are turned on. Do not use experimental features on live deployments. -// TypeError: (59-67): Reference types cannot be indexed. +// TypeError: (59-67): Indexed reference types cannot yet be used with ABIEncoderV2. diff --git a/test/libsolidity/syntaxTests/events/event_struct_indexed_v2.sol b/test/libsolidity/syntaxTests/events/event_struct_indexed_v2.sol index 8d2d4f8c4..a8e0837f8 100644 --- a/test/libsolidity/syntaxTests/events/event_struct_indexed_v2.sol +++ b/test/libsolidity/syntaxTests/events/event_struct_indexed_v2.sol @@ -5,4 +5,4 @@ contract c { } // ---- // Warning: (0-33): Experimental features are turned on. Do not use experimental features on live deployments. -// TypeError: (85-86): Reference types cannot be indexed. +// TypeError: (85-86): Indexed reference types cannot yet be used with ABIEncoderV2. From 409b20f204df74b24d77d9d88ea0fc9436c58d61 Mon Sep 17 00:00:00 2001 From: Leonardo Alt Date: Wed, 15 Aug 2018 17:10:02 +0200 Subject: [PATCH 4/5] Bug list entry --- docs/bugs.json | 8 ++++++++ docs/bugs_by_version.json | 26 ++++++++++++++++++++------ 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/docs/bugs.json b/docs/bugs.json index b464be18a..8eae2af37 100644 --- a/docs/bugs.json +++ b/docs/bugs.json @@ -1,4 +1,12 @@ [ + { + "name": "EventStructWrongData", + "summary": "Using structs in events logged wrong data.", + "description": "If a struct is used in an event, the address of the struct is logged instead of the actual data.", + "introduced": "0.4.17", + "fixed": "0.5.0", + "severity": "very low" + }, { "name": "OneOfTwoConstructorsSkipped", "summary": "If a contract has both a new-style constructor (using the constructor keyword) and an old-style constructor (a function with the same name as the contract) at the same time, one of them will be ignored.", diff --git a/docs/bugs_by_version.json b/docs/bugs_by_version.json index 2fe1d2266..00531b9e2 100644 --- a/docs/bugs_by_version.json +++ b/docs/bugs_by_version.json @@ -389,16 +389,21 @@ }, "0.4.17": { "bugs": [ + "EventStructWrongData", "ZeroFunctionSelector" ], "released": "2017-09-21" }, "0.4.18": { - "bugs": [], + "bugs": [ + "EventStructWrongData" + ], "released": "2017-10-18" }, "0.4.19": { - "bugs": [], + "bugs": [ + "EventStructWrongData" + ], "released": "2017-11-30" }, "0.4.2": { @@ -415,25 +420,34 @@ "released": "2016-09-17" }, "0.4.20": { - "bugs": [], + "bugs": [ + "EventStructWrongData" + ], "released": "2018-02-14" }, "0.4.21": { - "bugs": [], + "bugs": [ + "EventStructWrongData" + ], "released": "2018-03-07" }, "0.4.22": { "bugs": [ + "EventStructWrongData", "OneOfTwoConstructorsSkipped" ], "released": "2018-04-16" }, "0.4.23": { - "bugs": [], + "bugs": [ + "EventStructWrongData" + ], "released": "2018-04-19" }, "0.4.24": { - "bugs": [], + "bugs": [ + "EventStructWrongData" + ], "released": "2018-05-16" }, "0.4.3": { From 99022216cd19ecb777bf20c4bfff6fd12395c12e Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 6 Sep 2018 16:59:53 +0200 Subject: [PATCH 5/5] Change "fixed" date. --- docs/bugs.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/bugs.json b/docs/bugs.json index 8eae2af37..423c314d5 100644 --- a/docs/bugs.json +++ b/docs/bugs.json @@ -4,7 +4,7 @@ "summary": "Using structs in events logged wrong data.", "description": "If a struct is used in an event, the address of the struct is logged instead of the actual data.", "introduced": "0.4.17", - "fixed": "0.5.0", + "fixed": "0.4.25", "severity": "very low" }, {