Merge pull request #4841 from ethereum/struct_event_jsonpath

Buglist check script json path support

.circleci/config.yml

@@ -189,15 +189,21 @@ jobs:
           command: ./scripts/detect_trailing_whitespace.sh
 
   test_buglist:
-      docker:
+    docker:
-        - image: circleci/node
+      - image: circleci/node
-      environment:
+    environment:
-        TERM: xterm
+      TERM: xterm
-      steps:
+    steps:
-        - checkout
+      - checkout
-        - run:
+      - run:
-            name: Test buglist
+          name: JS deps
-            command: ./test/buglistTests.js
+          command: |
+            npm install download
+            npm install JSONPath
+            npm install mktemp
+      - run:
+          name: Test buglist
+          command: ./test/buglistTests.js
 
   test_x86_linux:
     docker:

docs/bugs.json

@@ -1,21 +1,22 @@
 [
-	{
+    {
-		"name": "EventStructWrongData",
+        "name": "EventStructWrongData",
-		"summary": "Using structs in events logged wrong data.",
+        "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.",
+        "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",
+        "introduced": "0.4.17",
-		"fixed": "0.5.0",
+        "fixed": "0.5.0",
-		"severity": "very low"
+        "severity": "very low",
-	},
+        "check": {"ast-compact-json-path": "$..[?(@.nodeType === 'EventDefinition')]..[?(@.nodeType === 'UserDefinedTypeName' && @.typeDescriptions.typeString.startsWith('struct'))]"}
-	{
+    },
-		"name": "NestedArrayFunctionCallDecoder",
+    {
-		"summary": "Calling functions that return multi-dimensional fixed-size arrays can result in memory corruption.",
+        "name": "NestedArrayFunctionCallDecoder",
-		"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.",
+        "summary": "Calling functions that return multi-dimensional fixed-size arrays can result in memory corruption.",
-		"introduced": "0.1.4",
+        "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.",
-		"fixed": "0.4.22",
+        "introduced": "0.1.4",
-		"severity": "medium",
+        "fixed": "0.4.22",
-		"check": {"regex-source": "returns[^;{]*\\[\\s*[^\\] \\t\\r\\n\\v\\f][^\\]]*\\]\\s*\\[\\s*[^\\] \\t\\r\\n\\v\\f][^\\]]*\\][^{;]*[;{]"}
+        "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.",

docs/bugs.rst

@@ -57,13 +57,18 @@ conditions
     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
+    This field contains different checks that report whether the smart contract
-    against the source code ("source-regex") to find out if the
+    contains the bug or not. The first type of check are Javascript regular
-    smart contract contains the bug or not. If there is no match,
+    expressions that are to be matched against the source code ("source-regex")
-    then the bug is very likely not present. If there is a match,
+    if the bug is present.  If there is no match, then the bug is very likely
-    the bug might be present. For improved accuracy, the regular
+    not present. If there is a match, the bug might be present.  For improved
-    expression should be applied to the source code after stripping
+    accuracy, the checks should be applied to the source code after stripping
     comments.
+    The second type of check are patterns to be checked on the compact AST of
+    the Solidity program ("ast-compact-json-path"). The specified search query
+    is a `JsonPath <https://github.com/json-path/JsonPath>`_ expression.
+    If at least one path of the Solidity AST matches the query, the bug is
+    likely present.
 
 .. literalinclude:: bugs.json
    :language: js

test/buglistTests.js

@@ -2,6 +2,11 @@
 
 "use strict";
 
+var util = require('util')
+var exec = util.promisify(require('child_process').exec)
+var mktemp = require('mktemp');
+var download = require('download')
+var JSONPath = require('JSONPath')
 var fs = require('fs')
 var bugs = JSON.parse(fs.readFileSync(__dirname + '/../docs/bugs.json', 'utf8'))
 
@@ -19,27 +24,111 @@ var tests = fs.readFileSync(__dirname + '/buglist_test_vectors.md', 'utf8')
 
 var testVectorParser = /\s*#\s+(\S+)\s+## buggy\n([^#]*)## fine\n([^#]*)/g
 
-var result;
+runTests()
-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'])
+async function runTests()
-    for (var i in buggy)
+{
-    {
+	var result;
-        if (!regex.exec(buggy[i]))
+	while ((result = testVectorParser.exec(tests)) !== null)
-        {
+	{
-            throw "Bug " + name + ": Buggy source does not match: " + buggy[i]
+		var name = result[1]
-        }
+		var buggy = result[2].split('\n--\n')
-    }
+		var fine = result[3].split('\n--\n')
-    for (var i in fine)
+		console.log("Testing " + name + " with " + buggy.length + " buggy and " + fine.length + " fine instances")
-    {
+
-        if (regex.exec(fine[i]))
+		try {
-        {
+			await checkRegex(name, buggy, fine)
-            throw "Bug " + name + ": Non-buggy source matches: " + fine[i]
+			await checkJSONPath(name, buggy, fine)
-        }
+		} catch (err) {
-    }
+			console.error("Error: " + err)
+		}
+	}
+}
+
+function checkRegex(name, buggy, fine)
+{
+    return new Promise(function(resolve, reject) {
+		var regexStr = bugsByName[name].check['regex-source']
+		if (regexStr !== undefined)
+		{
+			var regex = RegExp(regexStr)
+			for (var i in buggy)
+			{
+				if (!regex.exec(buggy[i]))
+				{
+					reject("Bug " + name + ": Buggy source does not match: " + buggy[i])
+				}
+			}
+			for (var i in fine)
+			{
+				if (regex.exec(fine[i]))
+				{
+					reject("Bug " + name + ": Non-buggy source matches: " + fine[i])
+				}
+			}
+		}
+		resolve()
+	})
+}
+
+async function checkJSONPath(name, buggy, fine)
+{
+	var jsonPath = bugsByName[name].check['ast-compact-json-path']
+	if (jsonPath !== undefined)
+	{
+		var url = "http://github.com/ethereum/solidity/releases/download/v" + bugsByName[name].introduced + "/solc-static-linux"
+		try {
+			var tmpdir = await mktemp.createDir('XXXXX')
+			var binary = tmpdir + "/solc-static-linux"
+			await download(url, tmpdir)
+			exec("chmod +x " + binary)
+			for (var i in buggy)
+			{
+				var result = await checkJsonPathTest(buggy[i], tmpdir, binary, jsonPath, i)
+				if (!result)
+					throw "Bug " + name + ": Buggy source does not contain path: " + buggy[i]
+			}
+			for (var i in fine)
+			{
+				var result = await checkJsonPathTest(fine[i], tmpdir, binary, jsonPath, i + buggy.length)
+				if (result)
+					throw "Bug " + name + ": Non-buggy source contains path: " + fine[i]
+			}
+			exec("rm -r " + tmpdir)
+		} catch (err) {
+			throw err
+		}
+	}
+}
+
+function checkJsonPathTest(code, tmpdir, binary, query, idx) {
+    return new Promise(function(resolve, reject) {
+        var solFile = tmpdir + "/jsonPath" + idx + ".sol"
+        var astFile = tmpdir + "/ast" + idx + ".json"
+        writeFilePromise(solFile, code)
+        .then(() => {
+            return exec(binary + " --ast-compact-json " + solFile + " > " + astFile)
+        })
+        .then(() => {
+            var jsonRE = /(\{[\s\S]*\})/
+            var ast = JSON.parse(jsonRE.exec(fs.readFileSync(astFile, 'utf8'))[0])
+            var result = JSONPath({json: ast, path: query})
+            if (result.length > 0)
+                resolve(true)
+            else
+                resolve(false)
+        })
+        .catch((err) => {
+            reject(err)
+        })
+    })
+}
+
+function writeFilePromise(filename, data) {
+    return new Promise(function(resolve, reject) {
+        fs.writeFile(filename, data, 'utf8', function(err) {
+            if (err) reject(err)
+            else resolve(data)
+        })
+    })
 }

test/buglist_test_vectors.md

@@ -67,3 +67,48 @@ function f() m(uint[2][2]) { }
 --
 
 function f() returns (uint, uint) { uint[2][2] memory x; }
+
+# EventStructWrongData
+
+## buggy
+
+pragma experimental ABIEncoderV2;
+contract C
+{
+	struct S { uint x; }
+	event E(S);
+	event F(S);
+	enum A { B, C }
+	event G(A);
+	function f(S s);
+}
+
+--
+
+pragma experimental ABIEncoderV2;
+contract C
+{
+	struct S { uint x; }
+	event E(S indexed);
+	event F(uint, S, bool);
+}
+
+## fine
+
+pragma experimental ABIEncoderV2;
+contract C
+{
+	struct S { uint x; }
+	enum A { B, C }
+	event G(A);
+}
+
+--
+
+pragma experimental ABIEncoderV2;
+contract C
+{
+	struct S { uint x; }
+	function f(S s);
+	S s1;
+}