diff options
| -rw-r--r-- | .circleci/config.yml | 12 | ||||
| -rw-r--r-- | Changelog.md | 1 | ||||
| -rw-r--r-- | docs/bugs.json | 9 | ||||
| -rw-r--r-- | docs/bugs.rst | 8 | ||||
| -rw-r--r-- | docs/bugs_by_version.json | 48 | ||||
| -rw-r--r-- | libdevcore/StringUtils.cpp | 5 | ||||
| -rw-r--r-- | libdevcore/StringUtils.h | 3 | ||||
| -rw-r--r-- | libsolidity/analysis/DeclarationContainer.cpp | 7 | ||||
| -rwxr-xr-x | test/buglistTests.js | 45 | ||||
| -rw-r--r-- | test/buglist_test_vectors.md | 69 | ||||
| -rw-r--r-- | test/libdevcore/StringUtils.cpp | 1 | ||||
| -rw-r--r-- | test/libsolidity/syntaxTests/nameAndTypeResolution/573_similar_name_longer_than_80_not_suggested.sol | 7 | ||||
| -rw-r--r-- | test/libsolidity/syntaxTests/nameAndTypeResolution/574_similar_name_shorter_than_80_suggested.sol | 7 |
13 files changed, 214 insertions, 8 deletions
diff --git a/.circleci/config.yml b/.circleci/config.yml index e3596d2b..4514626b 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -179,6 +179,17 @@ jobs: name: Check spelling command: ~/.local/bin/codespell -S "*.enc,.git" -I ./scripts/codespell_whitelist.txt + test_buglist: + docker: + - image: circleci/node + environment: + TERM: xterm + steps: + - checkout + - run: + name: Test buglist + command: ./test/buglistTests.js + test_x86_linux: docker: - image: buildpack-deps:artful @@ -252,6 +263,7 @@ workflows: build_all: jobs: - test_check_spelling: *build_on_tags + - test_buglist: *build_on_tags - build_emscripten: *build_on_tags - test_emscripten_solcjs: <<: *build_on_tags diff --git a/Changelog.md b/Changelog.md index 1a181cfe..177a071b 100644 --- a/Changelog.md +++ b/Changelog.md @@ -99,6 +99,7 @@ Bugfixes: * References Resolver: Enforce ``storage`` as data location for mappings. * References Resolver: Properly handle invalid references used together with ``_slot`` and ``_offset``. * References Resolver: Report error instead of assertion fail when FunctionType has an undeclared type as parameter. + * References Resolver: Fix high CPU usage when using large variable names issue. Only suggest similar name if identifiers shorter than 80 characters. * Type Checker: Default data location for type conversions (e.g. from literals) is memory and not storage. * Type Checker: Disallow assignments to mappings within tuple assignments as well. * Type Checker: Disallow packed encoding of arrays of structs. diff --git a/docs/bugs.json b/docs/bugs.json index b464be18..3f20077f 100644 --- a/docs/bugs.json +++ b/docs/bugs.json @@ -1,4 +1,13 @@ [ + { + "name": "NestedArrayFunctionCallDecoder", + "summary": "Calling functions that return multi-dimensional fixed-size arrays can result in memory corruption.", + "description": "If Solidity code calls a function that returns a multi-dimensional fixed-size array, array elements are incorrectly interpreted as memory pointers and thus can cause memory corruption if the return values are accessed. Calling functions with multi-dimensional fixed-size arrays is unaffected as is returning fixed-size arrays from function calls. The regular expression only checks if such functions are present, not if they are called, which is required for the contract to be affected.", + "introduced": "0.1.4", + "fixed": "0.4.22", + "severity": "medium", + "check": {"regex-source": "returns[^;{]*\\[\\s*[^\\] \\t\\r\\n\\v\\f][^\\]]*\\]\\s*\\[\\s*[^\\] \\t\\r\\n\\v\\f][^\\]]*\\][^{;]*[;{]"} + }, { "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.rst b/docs/bugs.rst index 7629830d..6f315a65 100644 --- a/docs/bugs.rst +++ b/docs/bugs.rst @@ -56,6 +56,14 @@ conditions is an object that can contain a boolean value ``optimizer``, which means that the optimizer has to be switched on to enable the bug. If no conditions are given, assume that the bug is present. +check + This field contains JavaScript regular expressions that are to be matched + against the source code ("source-regex") to find out if the + smart contract contains the bug or not. If there is no match, + then the bug is very likely not present. If there is a match, + the bug might be present. For improved accuracy, the regular + expression should be applied to the source code after stripping + comments. .. literalinclude:: bugs.json :language: js diff --git a/docs/bugs_by_version.json b/docs/bugs_by_version.json index 2fe1d226..b400121f 100644 --- a/docs/bugs_by_version.json +++ b/docs/bugs_by_version.json @@ -69,6 +69,7 @@ }, "0.1.4": { "bugs": [ + "NestedArrayFunctionCallDecoder", "ZeroFunctionSelector", "ECRecoverMalformedInput", "SkipEmptyStringLiteral", @@ -86,6 +87,7 @@ }, "0.1.5": { "bugs": [ + "NestedArrayFunctionCallDecoder", "ZeroFunctionSelector", "ECRecoverMalformedInput", "SkipEmptyStringLiteral", @@ -103,6 +105,7 @@ }, "0.1.6": { "bugs": [ + "NestedArrayFunctionCallDecoder", "ZeroFunctionSelector", "ECRecoverMalformedInput", "SkipEmptyStringLiteral", @@ -121,6 +124,7 @@ }, "0.1.7": { "bugs": [ + "NestedArrayFunctionCallDecoder", "ZeroFunctionSelector", "ECRecoverMalformedInput", "SkipEmptyStringLiteral", @@ -139,6 +143,7 @@ }, "0.2.0": { "bugs": [ + "NestedArrayFunctionCallDecoder", "ZeroFunctionSelector", "ECRecoverMalformedInput", "SkipEmptyStringLiteral", @@ -157,6 +162,7 @@ }, "0.2.1": { "bugs": [ + "NestedArrayFunctionCallDecoder", "ZeroFunctionSelector", "ECRecoverMalformedInput", "SkipEmptyStringLiteral", @@ -175,6 +181,7 @@ }, "0.2.2": { "bugs": [ + "NestedArrayFunctionCallDecoder", "ZeroFunctionSelector", "ECRecoverMalformedInput", "SkipEmptyStringLiteral", @@ -193,6 +200,7 @@ }, "0.3.0": { "bugs": [ + "NestedArrayFunctionCallDecoder", "ZeroFunctionSelector", "DelegateCallReturnValue", "ECRecoverMalformedInput", @@ -211,6 +219,7 @@ }, "0.3.1": { "bugs": [ + "NestedArrayFunctionCallDecoder", "ZeroFunctionSelector", "DelegateCallReturnValue", "ECRecoverMalformedInput", @@ -228,6 +237,7 @@ }, "0.3.2": { "bugs": [ + "NestedArrayFunctionCallDecoder", "ZeroFunctionSelector", "DelegateCallReturnValue", "ECRecoverMalformedInput", @@ -245,6 +255,7 @@ }, "0.3.3": { "bugs": [ + "NestedArrayFunctionCallDecoder", "ZeroFunctionSelector", "DelegateCallReturnValue", "ECRecoverMalformedInput", @@ -261,6 +272,7 @@ }, "0.3.4": { "bugs": [ + "NestedArrayFunctionCallDecoder", "ZeroFunctionSelector", "DelegateCallReturnValue", "ECRecoverMalformedInput", @@ -277,6 +289,7 @@ }, "0.3.5": { "bugs": [ + "NestedArrayFunctionCallDecoder", "ZeroFunctionSelector", "DelegateCallReturnValue", "ECRecoverMalformedInput", @@ -293,6 +306,7 @@ }, "0.3.6": { "bugs": [ + "NestedArrayFunctionCallDecoder", "ZeroFunctionSelector", "DelegateCallReturnValue", "ECRecoverMalformedInput", @@ -307,6 +321,7 @@ }, "0.4.0": { "bugs": [ + "NestedArrayFunctionCallDecoder", "ZeroFunctionSelector", "DelegateCallReturnValue", "ECRecoverMalformedInput", @@ -321,6 +336,7 @@ }, "0.4.1": { "bugs": [ + "NestedArrayFunctionCallDecoder", "ZeroFunctionSelector", "DelegateCallReturnValue", "ECRecoverMalformedInput", @@ -335,6 +351,7 @@ }, "0.4.10": { "bugs": [ + "NestedArrayFunctionCallDecoder", "ZeroFunctionSelector", "DelegateCallReturnValue", "ECRecoverMalformedInput", @@ -345,6 +362,7 @@ }, "0.4.11": { "bugs": [ + "NestedArrayFunctionCallDecoder", "ZeroFunctionSelector", "DelegateCallReturnValue", "ECRecoverMalformedInput", @@ -354,6 +372,7 @@ }, "0.4.12": { "bugs": [ + "NestedArrayFunctionCallDecoder", "ZeroFunctionSelector", "DelegateCallReturnValue", "ECRecoverMalformedInput" @@ -362,6 +381,7 @@ }, "0.4.13": { "bugs": [ + "NestedArrayFunctionCallDecoder", "ZeroFunctionSelector", "DelegateCallReturnValue", "ECRecoverMalformedInput" @@ -370,6 +390,7 @@ }, "0.4.14": { "bugs": [ + "NestedArrayFunctionCallDecoder", "ZeroFunctionSelector", "DelegateCallReturnValue" ], @@ -377,32 +398,40 @@ }, "0.4.15": { "bugs": [ + "NestedArrayFunctionCallDecoder", "ZeroFunctionSelector" ], "released": "2017-08-08" }, "0.4.16": { "bugs": [ + "NestedArrayFunctionCallDecoder", "ZeroFunctionSelector" ], "released": "2017-08-24" }, "0.4.17": { "bugs": [ + "NestedArrayFunctionCallDecoder", "ZeroFunctionSelector" ], "released": "2017-09-21" }, "0.4.18": { - "bugs": [], + "bugs": [ + "NestedArrayFunctionCallDecoder" + ], "released": "2017-10-18" }, "0.4.19": { - "bugs": [], + "bugs": [ + "NestedArrayFunctionCallDecoder" + ], "released": "2017-11-30" }, "0.4.2": { "bugs": [ + "NestedArrayFunctionCallDecoder", "ZeroFunctionSelector", "DelegateCallReturnValue", "ECRecoverMalformedInput", @@ -415,11 +444,15 @@ "released": "2016-09-17" }, "0.4.20": { - "bugs": [], + "bugs": [ + "NestedArrayFunctionCallDecoder" + ], "released": "2018-02-14" }, "0.4.21": { - "bugs": [], + "bugs": [ + "NestedArrayFunctionCallDecoder" + ], "released": "2018-03-07" }, "0.4.22": { @@ -438,6 +471,7 @@ }, "0.4.3": { "bugs": [ + "NestedArrayFunctionCallDecoder", "ZeroFunctionSelector", "DelegateCallReturnValue", "ECRecoverMalformedInput", @@ -450,6 +484,7 @@ }, "0.4.4": { "bugs": [ + "NestedArrayFunctionCallDecoder", "ZeroFunctionSelector", "DelegateCallReturnValue", "ECRecoverMalformedInput", @@ -461,6 +496,7 @@ }, "0.4.5": { "bugs": [ + "NestedArrayFunctionCallDecoder", "ZeroFunctionSelector", "DelegateCallReturnValue", "ECRecoverMalformedInput", @@ -473,6 +509,7 @@ }, "0.4.6": { "bugs": [ + "NestedArrayFunctionCallDecoder", "ZeroFunctionSelector", "DelegateCallReturnValue", "ECRecoverMalformedInput", @@ -484,6 +521,7 @@ }, "0.4.7": { "bugs": [ + "NestedArrayFunctionCallDecoder", "ZeroFunctionSelector", "DelegateCallReturnValue", "ECRecoverMalformedInput", @@ -494,6 +532,7 @@ }, "0.4.8": { "bugs": [ + "NestedArrayFunctionCallDecoder", "ZeroFunctionSelector", "DelegateCallReturnValue", "ECRecoverMalformedInput", @@ -504,6 +543,7 @@ }, "0.4.9": { "bugs": [ + "NestedArrayFunctionCallDecoder", "ZeroFunctionSelector", "DelegateCallReturnValue", "ECRecoverMalformedInput", diff --git a/libdevcore/StringUtils.cpp b/libdevcore/StringUtils.cpp index 01bdc675..50bf7cce 100644 --- a/libdevcore/StringUtils.cpp +++ b/libdevcore/StringUtils.cpp @@ -29,13 +29,16 @@ using namespace std; using namespace dev; -bool dev::stringWithinDistance(string const& _str1, string const& _str2, size_t _maxDistance) +bool dev::stringWithinDistance(string const& _str1, string const& _str2, size_t _maxDistance, size_t _lenThreshold) { if (_str1 == _str2) return true; size_t n1 = _str1.size(); size_t n2 = _str2.size(); + if (_lenThreshold > 0 && n1 * n2 > _lenThreshold) + return false; + size_t distance = stringDistance(_str1, _str2); // if distance is not greater than _maxDistance, and distance is strictly less than length of both names, they can be considered similar diff --git a/libdevcore/StringUtils.h b/libdevcore/StringUtils.h index f05a426b..b02b9d12 100644 --- a/libdevcore/StringUtils.h +++ b/libdevcore/StringUtils.h @@ -30,7 +30,8 @@ namespace dev { // Calculates the Damerau–Levenshtein distance between _str1 and _str2 and returns true if that distance is not greater than _maxDistance -bool stringWithinDistance(std::string const& _str1, std::string const& _str2, size_t _maxDistance); +// if _lenThreshold > 0 and the product of the strings length is greater than _lenThreshold, the function will return false +bool stringWithinDistance(std::string const& _str1, std::string const& _str2, size_t _maxDistance, size_t _lenThreshold = 0); // Calculates the Damerau–Levenshtein distance between _str1 and _str2 size_t stringDistance(std::string const& _str1, std::string const& _str2); // Return a string having elements of suggestions as quoted, alternative suggestions. e.g. "a", "b" or "c" diff --git a/libsolidity/analysis/DeclarationContainer.cpp b/libsolidity/analysis/DeclarationContainer.cpp index 347daaf8..5f980788 100644 --- a/libsolidity/analysis/DeclarationContainer.cpp +++ b/libsolidity/analysis/DeclarationContainer.cpp @@ -138,19 +138,22 @@ vector<Declaration const*> DeclarationContainer::resolveName(ASTString const& _n vector<ASTString> DeclarationContainer::similarNames(ASTString const& _name) const { static size_t const MAXIMUM_EDIT_DISTANCE = 2; + // because the function below has quadratic runtime - it will not magically improve once a better algorithm is discovered ;) + // since 80 is the suggested line length limit, we use 80^2 as length threshold + static size_t const MAXIMUM_LENGTH_THRESHOLD = 80 * 80; vector<ASTString> similar; for (auto const& declaration: m_declarations) { string const& declarationName = declaration.first; - if (stringWithinDistance(_name, declarationName, MAXIMUM_EDIT_DISTANCE)) + if (stringWithinDistance(_name, declarationName, MAXIMUM_EDIT_DISTANCE, MAXIMUM_LENGTH_THRESHOLD)) similar.push_back(declarationName); } for (auto const& declaration: m_invisibleDeclarations) { string const& declarationName = declaration.first; - if (stringWithinDistance(_name, declarationName, MAXIMUM_EDIT_DISTANCE)) + if (stringWithinDistance(_name, declarationName, MAXIMUM_EDIT_DISTANCE, MAXIMUM_LENGTH_THRESHOLD)) similar.push_back(declarationName); } diff --git a/test/buglistTests.js b/test/buglistTests.js new file mode 100755 index 00000000..6b7df2f2 --- /dev/null +++ b/test/buglistTests.js @@ -0,0 +1,45 @@ +#!/usr/bin/env node + +"use strict"; + +var fs = require('fs') +var bugs = JSON.parse(fs.readFileSync(__dirname + '/../docs/bugs.json', 'utf8')) + +var bugsByName = {} +for (var i in bugs) +{ + if (bugs[i].name in bugsByName) + { + throw "Duplicate bug name: " + bugs[i].name + } + bugsByName[bugs[i].name] = bugs[i] +} + +var tests = fs.readFileSync(__dirname + '/buglist_test_vectors.md', 'utf8') + +var testVectorParser = /\s*#\s+(\S+)\s+## buggy\n([^#]*)## fine\n([^#]*)/g + +var result; +while ((result = testVectorParser.exec(tests)) !== null) +{ + var name = result[1] + var buggy = result[2].split('\n--\n') + var fine = result[3].split('\n--\n') + console.log("Testing " + name + " with " + buggy.length + " buggy and " + fine.length + " fine instances") + + var regex = RegExp(bugsByName[name].check['regex-source']) + for (var i in buggy) + { + if (!regex.exec(buggy[i])) + { + throw "Bug " + name + ": Buggy source does not match: " + buggy[i] + } + } + for (var i in fine) + { + if (regex.exec(fine[i])) + { + throw "Bug " + name + ": Non-buggy source matches: " + fine[i] + } + } +} diff --git a/test/buglist_test_vectors.md b/test/buglist_test_vectors.md new file mode 100644 index 00000000..ce95403b --- /dev/null +++ b/test/buglist_test_vectors.md @@ -0,0 +1,69 @@ +# NestedArrayFunctionCallDecoder + +## buggy + +function f() pure returns (uint[2][2]) { } + +-- + +function f() returns (uint[2][2] a) { } + +-- + +function f() returns (uint x, uint[200][2] a) { } + +-- + +function f() returns (uint[200][2] a, uint x) { } + +-- + +function f() returns (uint[200][2] a, uint x); + +-- + +function f() returns ( + uint + [ + 200 + ] + [2] + a, uint x); + +-- + +function f() returns ( + uint + [ + ContractName.ConstantName + ] + [2] + a, uint x); + +## fine + +function f() returns (uint[2]) { } + +-- + +function f() public pure returns (uint[2][] a) { } + +-- + +function f() public pure returns (uint[ 2 ] [ ] a) { } + +-- + +function f() public pure returns (uint x, uint[] a) { } + +-- + +function f(uint[2][2]) { } + +-- + +function f() m(uint[2][2]) { } + +-- + +function f() returns (uint, uint) { uint[2][2] memory x; } diff --git a/test/libdevcore/StringUtils.cpp b/test/libdevcore/StringUtils.cpp index 94f1b753..76c11b82 100644 --- a/test/libdevcore/StringUtils.cpp +++ b/test/libdevcore/StringUtils.cpp @@ -48,6 +48,7 @@ BOOST_AUTO_TEST_CASE(test_similarity) BOOST_CHECK_EQUAL(stringWithinDistance("abc", "abcdef", 2), false); BOOST_CHECK_EQUAL(stringWithinDistance("abcd", "wxyz", 2), false); BOOST_CHECK_EQUAL(stringWithinDistance("", "", 2), true); + BOOST_CHECK_EQUAL(stringWithinDistance("YYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYY", "YYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYZ", 2, 6400), false); } BOOST_AUTO_TEST_CASE(test_dldistance) diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/573_similar_name_longer_than_80_not_suggested.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/573_similar_name_longer_than_80_not_suggested.sol new file mode 100644 index 00000000..c6719d8c --- /dev/null +++ b/test/libsolidity/syntaxTests/nameAndTypeResolution/573_similar_name_longer_than_80_not_suggested.sol @@ -0,0 +1,7 @@ +contract test { + function f() public { + int YYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYY = YYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYY; + } +} +// ---- +// DeclarationError: (146-236): Undeclared identifier. diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/574_similar_name_shorter_than_80_suggested.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/574_similar_name_shorter_than_80_suggested.sol new file mode 100644 index 00000000..61fb2d82 --- /dev/null +++ b/test/libsolidity/syntaxTests/nameAndTypeResolution/574_similar_name_shorter_than_80_suggested.sol @@ -0,0 +1,7 @@ +contract test { + function f() public { + int YYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYY = YYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYY; + } +} +// ---- +// DeclarationError: (137-216): Undeclared identifier. Did you mean "YYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYY"?
\ No newline at end of file |