Skip to content

Commit 9927964

Browse files
author
Leonardo Alt
committed
Buglist check script supports json paths
1 parent c57a608 commit 9927964

File tree

5 files changed

+199
-53
lines changed

5 files changed

+199
-53
lines changed

‎.circleci/config.yml

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -189,15 +189,21 @@ jobs:
189189
command: ./scripts/detect_trailing_whitespace.sh
190190

191191
test_buglist:
192-
docker:
193-
- image: circleci/node
194-
environment:
195-
TERM: xterm
196-
steps:
197-
- checkout
198-
- run:
199-
name: Test buglist
200-
command: ./test/buglistTests.js
192+
docker:
193+
- image: circleci/node
194+
environment:
195+
TERM: xterm
196+
steps:
197+
- checkout
198+
- run:
199+
name: JS deps
200+
command: |
201+
npm install download
202+
npm install JSONPath
203+
npm install mktemp
204+
- run:
205+
name: Test buglist
206+
command: ./test/buglistTests.js
201207

202208
test_x86_linux:
203209
docker:

‎docs/bugs.json

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,22 @@
11
[
2-
{
3-
"name": "EventStructWrongData",
4-
"summary": "Using structs in events logged wrong data.",
5-
"description": "If a struct is used in an event, the address of the struct is logged instead of the actual data.",
6-
"introduced": "0.4.17",
7-
"fixed": "0.5.0",
8-
"severity": "very low"
9-
},
10-
{
11-
"name": "NestedArrayFunctionCallDecoder",
12-
"summary": "Calling functions that return multi-dimensional fixed-size arrays can result in memory corruption.",
13-
"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.",
14-
"introduced": "0.1.4",
15-
"fixed": "0.4.22",
16-
"severity": "medium",
17-
"check": {"regex-source": "returns[^;{]*\\[\\s*[^\\] \\t\\r\\n\\v\\f][^\\]]*\\]\\s*\\[\\s*[^\\] \\t\\r\\n\\v\\f][^\\]]*\\][^{;]*[;{]"}
18-
},
2+
{
3+
"name": "EventStructWrongData",
4+
"summary": "Using structs in events logged wrong data.",
5+
"description": "If a struct is used in an event, the address of the struct is logged instead of the actual data.",
6+
"introduced": "0.4.17",
7+
"fixed": "0.5.0",
8+
"severity": "very low",
9+
"check": {"ast-compact-json-path": "$..[?(@.nodeType === 'EventDefinition')]..[?(@.nodeType === 'UserDefinedTypeName' && @.typeDescriptions.typeString.startsWith('struct'))]"}
10+
},
11+
{
12+
"name": "NestedArrayFunctionCallDecoder",
13+
"summary": "Calling functions that return multi-dimensional fixed-size arrays can result in memory corruption.",
14+
"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.",
15+
"introduced": "0.1.4",
16+
"fixed": "0.4.22",
17+
"severity": "medium",
18+
"check": {"regex-source": "returns[^;{]*\\[\\s*[^\\] \\t\\r\\n\\v\\f][^\\]]*\\]\\s*\\[\\s*[^\\] \\t\\r\\n\\v\\f][^\\]]*\\][^{;]*[;{]"}
19+
},
1920
{
2021
"name": "OneOfTwoConstructorsSkipped",
2122
"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

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -57,13 +57,18 @@ conditions
5757
means that the optimizer has to be switched on to enable the bug.
5858
If no conditions are given, assume that the bug is present.
5959
check
60-
This field contains JavaScript regular expressions that are to be matched
61-
against the source code ("source-regex") to find out if the
62-
smart contract contains the bug or not. If there is no match,
63-
then the bug is very likely not present. If there is a match,
64-
the bug might be present. For improved accuracy, the regular
65-
expression should be applied to the source code after stripping
60+
This field contains different checks that report whether the smart contract
61+
contains the bug or not. The first type of check are Javascript regular
62+
expressions that are to be matched against the source code ("source-regex")
63+
if the bug is present. If there is no match, then the bug is very likely
64+
not present. If there is a match, the bug might be present. For improved
65+
accuracy, the checks should be applied to the source code after stripping
6666
comments.
67+
The second type of check are patterns to be checked on the compact AST of
68+
the Solidity program ("ast-compact-json-path"). The specified search query
69+
is a `JsonPath <https://github.com/json-path/JsonPath>`_ expression.
70+
If at least one path of the Solidity AST matches the query, the bug is
71+
likely present.
6772

6873
.. literalinclude:: bugs.json
6974
:language: js

‎test/buglistTests.js

Lines changed: 110 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,11 @@
22

33
"use strict";
44

5+
var util = require('util')
6+
var exec = util.promisify(require('child_process').exec)
7+
var mktemp = require('mktemp');
8+
var download = require('download')
9+
var JSONPath = require('JSONPath')
510
var fs = require('fs')
611
var bugs = JSON.parse(fs.readFileSync(__dirname + '/../docs/bugs.json', 'utf8'))
712

@@ -19,27 +24,111 @@ var tests = fs.readFileSync(__dirname + '/buglist_test_vectors.md', 'utf8')
1924

2025
var testVectorParser = /\s*#\s+(\S+)\s+## buggy\n([^#]*)## fine\n([^#]*)/g
2126

22-
var result;
23-
while ((result = testVectorParser.exec(tests)) !== null)
27+
runTests()
28+
29+
async function runTests()
2430
{
25-
var name = result[1]
26-
var buggy = result[2].split('\n--\n')
27-
var fine = result[3].split('\n--\n')
28-
console.log("Testing " + name + " with " + buggy.length + " buggy and " + fine.length + " fine instances")
31+
var result;
32+
while ((result = testVectorParser.exec(tests)) !== null)
33+
{
34+
var name = result[1]
35+
var buggy = result[2].split('\n--\n')
36+
var fine = result[3].split('\n--\n')
37+
console.log("Testing " + name + " with " + buggy.length + " buggy and " + fine.length + " fine instances")
2938

30-
var regex = RegExp(bugsByName[name].check['regex-source'])
31-
for (var i in buggy)
32-
{
33-
if (!regex.exec(buggy[i]))
34-
{
35-
throw "Bug " + name + ": Buggy source does not match: " + buggy[i]
36-
}
37-
}
38-
for (var i in fine)
39-
{
40-
if (regex.exec(fine[i]))
41-
{
42-
throw "Bug " + name + ": Non-buggy source matches: " + fine[i]
43-
}
44-
}
39+
try {
40+
await checkRegex(name, buggy, fine)
41+
await checkJSONPath(name, buggy, fine)
42+
} catch (err) {
43+
console.error("Error: " + err)
44+
}
45+
}
46+
}
47+
48+
function checkRegex(name, buggy, fine)
49+
{
50+
return new Promise(function(resolve, reject) {
51+
var regexStr = bugsByName[name].check['regex-source']
52+
if (regexStr !== undefined)
53+
{
54+
var regex = RegExp(regexStr)
55+
for (var i in buggy)
56+
{
57+
if (!regex.exec(buggy[i]))
58+
{
59+
reject("Bug " + name + ": Buggy source does not match: " + buggy[i])
60+
}
61+
}
62+
for (var i in fine)
63+
{
64+
if (regex.exec(fine[i]))
65+
{
66+
reject("Bug " + name + ": Non-buggy source matches: " + fine[i])
67+
}
68+
}
69+
}
70+
resolve()
71+
})
72+
}
73+
74+
async function checkJSONPath(name, buggy, fine)
75+
{
76+
var jsonPath = bugsByName[name].check['ast-compact-json-path']
77+
if (jsonPath !== undefined)
78+
{
79+
var url = "http://github.com/ethereum/solidity/releases/download/v" + bugsByName[name].introduced + "/solc-static-linux"
80+
try {
81+
var tmpdir = await mktemp.createDir('XXXXX')
82+
var binary = tmpdir + "/solc-static-linux"
83+
await download(url, tmpdir)
84+
exec("chmod +x " + binary)
85+
for (var i in buggy)
86+
{
87+
var result = await checkJsonPathTest(buggy[i], tmpdir, binary, jsonPath, i)
88+
if (!result)
89+
throw "Bug " + name + ": Buggy source does not contain path: " + buggy[i]
90+
}
91+
for (var i in fine)
92+
{
93+
var result = await checkJsonPathTest(fine[i], tmpdir, binary, jsonPath, i + buggy.length)
94+
if (result)
95+
throw "Bug " + name + ": Non-buggy source contains path: " + fine[i]
96+
}
97+
exec("rm -r " + tmpdir)
98+
} catch (err) {
99+
throw err
100+
}
101+
}
102+
}
103+
104+
function checkJsonPathTest(code, tmpdir, binary, query, idx) {
105+
return new Promise(function(resolve, reject) {
106+
var solFile = tmpdir + "/jsonPath" + idx + ".sol"
107+
var astFile = tmpdir + "/ast" + idx + ".json"
108+
writeFilePromise(solFile, code)
109+
.then(() => {
110+
return exec(binary + " --ast-compact-json " + solFile + " > " + astFile)
111+
})
112+
.then(() => {
113+
var jsonRE = /(\{[\s\S]*\})/
114+
var ast = JSON.parse(jsonRE.exec(fs.readFileSync(astFile, 'utf8'))[0])
115+
var result = JSONPath({json: ast, path: query})
116+
if (result.length > 0)
117+
resolve(true)
118+
else
119+
resolve(false)
120+
})
121+
.catch((err) => {
122+
reject(err)
123+
})
124+
})
125+
}
126+
127+
function writeFilePromise(filename, data) {
128+
return new Promise(function(resolve, reject) {
129+
fs.writeFile(filename, data, 'utf8', function(err) {
130+
if (err) reject(err)
131+
else resolve(data)
132+
})
133+
})
45134
}

‎test/buglist_test_vectors.md

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,3 +67,48 @@ function f() m(uint[2][2]) { }
6767
--
6868

6969
function f() returns (uint, uint) { uint[2][2] memory x; }
70+
71+
# EventStructWrongData
72+
73+
## buggy
74+
75+
pragma experimental ABIEncoderV2;
76+
contract C
77+
{
78+
struct S { uint x; }
79+
event E(S);
80+
event F(S);
81+
enum A { B, C }
82+
event G(A);
83+
function f(S s);
84+
}
85+
86+
--
87+
88+
pragma experimental ABIEncoderV2;
89+
contract C
90+
{
91+
struct S { uint x; }
92+
event E(S indexed);
93+
event F(uint, S, bool);
94+
}
95+
96+
## fine
97+
98+
pragma experimental ABIEncoderV2;
99+
contract C
100+
{
101+
struct S { uint x; }
102+
enum A { B, C }
103+
event G(A);
104+
}
105+
106+
--
107+
108+
pragma experimental ABIEncoderV2;
109+
contract C
110+
{
111+
struct S { uint x; }
112+
function f(S s);
113+
S s1;
114+
}

0 commit comments

Comments
 (0)