From 44327da285c3f4ec77b403905b8e15429efdca26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Fr=C3=B6wis?= Date: Mon, 3 Apr 2017 19:05:31 +0200 Subject: [PATCH] Static Analysis: integration tests --- src/app/staticanalysis/astWalker.js | 54 ++++ .../staticanalysis/modules/abstractAstView.js | 3 +- .../modules/checksEffectsInteraction.js | 2 + .../modules/constantFunctions.js | 2 + src/app/staticanalysis/modules/list.js | 3 + .../modules/staticAnalysisCommon.js | 8 +- .../staticanalysis/staticAnalysisRunner.js | 3 +- .../staticAnalysisCommon-test.js | 9 + .../staticAnalysisIntegration-test.js | 239 ++++++++++++++---- .../test-contracts/assembly.sol | 15 +- 10 files changed, 281 insertions(+), 57 deletions(-) create mode 100644 src/app/staticanalysis/astWalker.js diff --git a/src/app/staticanalysis/astWalker.js b/src/app/staticanalysis/astWalker.js new file mode 100644 index 0000000000..f8737122d6 --- /dev/null +++ b/src/app/staticanalysis/astWalker.js @@ -0,0 +1,54 @@ +'use strict' +/** + * Crawl the given AST through the function walk(ast, callback) + */ +function AstWalker () { +} + +/** + * visit all the AST nodes + * + * @param {Object} ast - AST node + * @param {Object or Function} callback - if (Function) the function will be called for every node. + * - if (Object) callback[] will be called for + * every node of type . callback["*"] will be called fo all other nodes. + * in each case, if the callback returns false it does not descend into children. + * If no callback for the current type, children are visited. + */ +AstWalker.prototype.walk = function (ast, callback) { + if (callback instanceof Function) { + callback = {'*': callback} + } + if (!('*' in callback)) { + callback['*'] = function () { return true } + } + if (manageCallBack(ast, callback) && ast.children && ast.children.length > 0) { + for (var k in ast.children) { + var child = ast.children[k] + this.walk(child, callback) + } + } +} + +/** + * walk the given @astList + * + * @param {Object} sourcesList - sources list (containing root AST node) + * @param {Function} - callback used by AstWalker to compute response + */ +AstWalker.prototype.walkAstList = function (sourcesList, callback) { + var walker = new AstWalker() + for (var k in sourcesList) { + walker.walk(sourcesList[k].AST, callback) + } +} + +function manageCallBack (node, callback) { + if (node.name in callback) { + return callback[node.name](node) + } else { + return callback['*'](node) + } +} + +module.exports = AstWalker diff --git a/src/app/staticanalysis/modules/abstractAstView.js b/src/app/staticanalysis/modules/abstractAstView.js index 7f2a610a45..0d7cc42bca 100644 --- a/src/app/staticanalysis/modules/abstractAstView.js +++ b/src/app/staticanalysis/modules/abstractAstView.js @@ -1,5 +1,6 @@ var common = require('./staticAnalysisCommon') -var AstWalker = require('ethereum-remix').util.AstWalker +// var AstWalker = require('ethereum-remix').util.AstWalker +var AstWalker = require('../astWalker') function abstractAstView () { this.contracts = null diff --git a/src/app/staticanalysis/modules/checksEffectsInteraction.js b/src/app/staticanalysis/modules/checksEffectsInteraction.js index 86f60a2545..7145e48d17 100644 --- a/src/app/staticanalysis/modules/checksEffectsInteraction.js +++ b/src/app/staticanalysis/modules/checksEffectsInteraction.js @@ -19,6 +19,8 @@ checksEffectsInteraction.prototype.report = function (compilationResults) { var cg = callGraph.buildGlobalFuncCallGraph(this.contracts) + if (this.contracts.some((item) => item.modifiers.length > 0)) this.warning.push({ warning: `Modifiers are currently not supported by this analysis.` }) + this.contracts.forEach((contract) => { contract.functions.forEach((func) => { func.changesState = checkIfChangesState(common.getFullQuallyfiedFuncDefinitionIdent(contract.node, func.node, func.parameters), diff --git a/src/app/staticanalysis/modules/constantFunctions.js b/src/app/staticanalysis/modules/constantFunctions.js index c1c95a9399..8f69e204c8 100644 --- a/src/app/staticanalysis/modules/constantFunctions.js +++ b/src/app/staticanalysis/modules/constantFunctions.js @@ -19,6 +19,8 @@ constantFunctions.prototype.report = function (compilationResults) { var cg = callGraph.buildGlobalFuncCallGraph(this.contracts) + if (this.contracts.some((item) => item.modifiers.length > 0)) this.warning.push({ warning: `Modifiers are currently not supported by this analysis.` }) + this.contracts.forEach((contract) => { if (!common.isFullyImplementedContract(contract.node)) return diff --git a/src/app/staticanalysis/modules/list.js b/src/app/staticanalysis/modules/list.js index 57ed694688..a27238f8bb 100644 --- a/src/app/staticanalysis/modules/list.js +++ b/src/app/staticanalysis/modules/list.js @@ -5,4 +5,7 @@ module.exports = [ require('./checksEffectsInteraction'), require('./constantFunctions'), require('./inlineAssembly') + // require('./blockTimestamp'), + // require('./lowLevelCalls'), + // require('./blockBlockhash') ] diff --git a/src/app/staticanalysis/modules/staticAnalysisCommon.js b/src/app/staticanalysis/modules/staticAnalysisCommon.js index 8c4190f840..8d01e8719e 100644 --- a/src/app/staticanalysis/modules/staticAnalysisCommon.js +++ b/src/app/staticanalysis/modules/staticAnalysisCommon.js @@ -226,18 +226,21 @@ function isExternalDirectCall (node) { return isMemberAccess(node, basicRegex.EXTERNALFUNCTIONTYPE, undefined, basicRegex.CONTRACTTYPE, undefined) && !isThisLocalCall(node) } -// usage of now special variable function isNowAccess (node) { return nodeType(node, exactMatch(nodeTypes.IDENTIFIER)) && expressionType(node, exactMatch(basicTypes.UINT)) && name(node, exactMatch('now')) } -// usage of block timestamp function isBlockTimestampAccess (node) { return isSpecialVariableAccess(node, specialVariables.BLOCKTIMESTAMP) } +// usage of block timestamp +function isBlockBlockHashAccess (node) { + return isSpecialVariableAccess(node, specialVariables.BLOCKHASH) +} + function isThisLocalCall (node) { return isMemberAccess(node, basicRegex.FUNCTIONTYPE, exactMatch('this'), basicRegex.CONTRACTTYPE, undefined) } @@ -382,6 +385,7 @@ module.exports = { isEffect: isEffect, isNowAccess: isNowAccess, isBlockTimestampAccess: isBlockTimestampAccess, + isBlockBlockHashAccess: isBlockBlockHashAccess, isThisLocalCall: isThisLocalCall, isLocalCall: isLocalCall, isWriteOnStateVariable: isWriteOnStateVariable, diff --git a/src/app/staticanalysis/staticAnalysisRunner.js b/src/app/staticanalysis/staticAnalysisRunner.js index 0583fb30d4..c3855aea0e 100644 --- a/src/app/staticanalysis/staticAnalysisRunner.js +++ b/src/app/staticanalysis/staticAnalysisRunner.js @@ -1,5 +1,6 @@ 'use strict' -var AstWalker = require('ethereum-remix').util.AstWalker +// var AstWalker = require('ethereum-remix').util.AstWalker +var AstWalker = require('./astWalker') var list = require('./modules/list') function staticAnalysisRunner () { diff --git a/test/staticanalysis/staticAnalysisCommon-test.js b/test/staticanalysis/staticAnalysisCommon-test.js index 9d1e85ea2d..cb2396f7be 100644 --- a/test/staticanalysis/staticAnalysisCommon-test.js +++ b/test/staticanalysis/staticAnalysisCommon-test.js @@ -1593,6 +1593,15 @@ test('staticAnalysisCommon.isBlockTimestampAccess', function (t) { t.notOk(common.isNowAccess(node), 'is now used should not work') }) +test('staticAnalysisCommon.isBlockBlockhashAccess', function (t) { + t.plan(4) + var node = { name: 'MemberAccess', children: [{attributes: { value: 'block', type: 'block' }}], attributes: { value: 'blockhash', type: 'bytes32' } } + t.notOk(common.isThisLocalCall(node), 'is this.local_method() used should not work') + t.notOk(common.isBlockTimestampAccess(node), 'is block.timestamp used should not work') + t.ok(common.isBlockBlockHashAccess(node), 'blockhash should work') + t.notOk(common.isNowAccess(node), 'is now used should not work') +}) + test('staticAnalysisCommon.isThisLocalCall', function (t) { t.plan(3) var node = { name: 'MemberAccess', children: [{attributes: { value: 'this', type: 'contract test' }}], attributes: { value: 'b', type: 'function (bytes32,address) returns (bool)' } } diff --git a/test/staticanalysis/staticAnalysisIntegration-test.js b/test/staticanalysis/staticAnalysisIntegration-test.js index d784bddc67..036d212f6d 100644 --- a/test/staticanalysis/staticAnalysisIntegration-test.js +++ b/test/staticanalysis/staticAnalysisIntegration-test.js @@ -1,47 +1,192 @@ -// var test = require('tape') - -// var common = require('../../babelify-src/app/staticanalysis/modules/staticAnalysisCommon') -// var StatRunner = require('../../babelify-src/app/staticanalysis/staticAnalysisRunner') -// var utils = require('../../babelify-src/app/utils') -// var Compiler = require('../../babelify-src/app/compiler') - -// var fs = require('fs') -// var path = require('path') - -// var testFiles = [ -// '/test-contracts/KingOfTheEtherThrone.sol', -// '/test-contracts/assembly.sol', -// '/test-contracts/ballot.sol', -// '/test-contracts/ballot_reentrant.sol', -// '/test-contracts/ballot_withoutWarning.sol', -// '/test-contracts/cross_contract.sol', -// '/test-contracts/inheritance.sol', -// '/test-contracts/notReentrant.sol', -// '/test-contracts/structReentrant.sol', -// '/test-contracts/thisLocal.sol', -// '/test-contracts/modifier1.sol', -// '/test-contracts/modifier2.sol' -// ] - -// test('thisLocal.js', function (t) { -// t.plan(0) - -// var module = require('../../babelify-src/app/staticanalysis/modules/thisLocal') - -// runModuleOnFiles(module, t) -// }) - -// function runModuleOnFiles (module, t) { -// var fakeImport = function (url, cb) { cb('Not implemented') } -// var compiler = new Compiler(fakeImport) -// var statRunner = new StatRunner() - -// testFiles.map((fileName) => { -// var contents = fs.readFileSync(path.join(__dirname, fileName), 'utf8') -// var compres = compiler.compile({ 'test': contents }, 'test') - -// statRunner.runWithModuleList(compres, [{ name: module.name, mod: new module.Module() }], (reports) => { -// reports.map((r) => t.comment(r.warning)) -// }) -// }) -// } +var test = require('tape') + +var StatRunner = require('../../babelify-src/app/staticanalysis/staticAnalysisRunner') +// const util = require('util') + +var solc = require('solc') + +var fs = require('fs') +var path = require('path') + +var testFiles = [ + 'KingOfTheEtherThrone.sol', + 'assembly.sol', + 'ballot.sol', + 'ballot_reentrant.sol', + 'ballot_withoutWarnings.sol', + 'cross_contract.sol', + 'inheritance.sol', + 'modifier1.sol', + 'modifier2.sol', + 'notReentrant.sol', + 'structReentrant.sol', + 'thisLocal.sol' +] + +var testFileAsts = {} + +testFiles.forEach((fileName) => { + var contents = fs.readFileSync(path.join(__dirname, 'test-contracts', fileName), 'utf8') + testFileAsts[fileName] = solc.compile(contents, 0) +}) + +test('Integration test thisLocal.js', function (t) { + t.plan(testFiles.length) + + var module = require('../../babelify-src/app/staticanalysis/modules/thisLocal') + + var lengthCheck = { + 'KingOfTheEtherThrone.sol': 0, + 'assembly.sol': 0, + 'ballot.sol': 0, + 'ballot_reentrant.sol': 1, + 'ballot_withoutWarnings.sol': 0, + 'cross_contract.sol': 0, + 'inheritance.sol': 0, + 'modifier1.sol': 0, + 'modifier2.sol': 0, + 'notReentrant.sol': 0, + 'structReentrant.sol': 0, + 'thisLocal.sol': 1 + } + + runModuleOnFiles(module, t, (file, report) => { + t.equal(report.length, lengthCheck[file], `${file} has right amount of this local warnings`) + }) +}) + +test('Integration test checksEffectsInteraction.js', function (t) { + t.plan(testFiles.length) + + var module = require('../../babelify-src/app/staticanalysis/modules/checksEffectsInteraction') + + var lengthCheck = { + 'KingOfTheEtherThrone.sol': 1, + 'assembly.sol': 0, + 'ballot.sol': 0, + 'ballot_reentrant.sol': 1, + 'ballot_withoutWarnings.sol': 0, + 'cross_contract.sol': 0, + 'inheritance.sol': 1, + 'modifier1.sol': 0, + 'modifier2.sol': 0, + 'notReentrant.sol': 0, + 'structReentrant.sol': 1, + 'thisLocal.sol': 0 + } + + runModuleOnFiles(module, t, (file, report) => { + t.equal(report.length, lengthCheck[file], `${file} has right amount of checks-effects-interaction warnings`) + }) +}) + +test('Integration test constatnFunctions.js', function (t) { + t.plan(testFiles.length) + + var module = require('../../babelify-src/app/staticanalysis/modules/constantFunctions') + + var lengthCheck = { + 'KingOfTheEtherThrone.sol': 0, + 'assembly.sol': 0, + 'ballot.sol': 0, + 'ballot_reentrant.sol': 0, + 'ballot_withoutWarnings.sol': 0, + 'cross_contract.sol': 1, + 'inheritance.sol': 0, + 'modifier1.sol': 1, + 'modifier2.sol': 0, + 'notReentrant.sol': 0, + 'structReentrant.sol': 0, + 'thisLocal.sol': 1 + } + + runModuleOnFiles(module, t, (file, report) => { + t.equal(report.length, lengthCheck[file], `${file} has right amount of constant warnings`) + }) +}) + +test('Integration test constantFunctions.js', function (t) { + t.plan(testFiles.length) + + var module = require('../../babelify-src/app/staticanalysis/modules/inlineAssembly') + + var lengthCheck = { + 'KingOfTheEtherThrone.sol': 0, + 'assembly.sol': 2, + 'ballot.sol': 0, + 'ballot_reentrant.sol': 0, + 'ballot_withoutWarnings.sol': 0, + 'cross_contract.sol': 0, + 'inheritance.sol': 0, + 'modifier1.sol': 0, + 'modifier2.sol': 0, + 'notReentrant.sol': 0, + 'structReentrant.sol': 0, + 'thisLocal.sol': 0 + } + + runModuleOnFiles(module, t, (file, report) => { + t.equal(report.length, lengthCheck[file], `${file} has right amount of inline assembly warnings`) + }) +}) + +test('Integration test txOrigin.js', function (t) { + t.plan(testFiles.length) + + var module = require('../../babelify-src/app/staticanalysis/modules/txOrigin') + + var lengthCheck = { + 'KingOfTheEtherThrone.sol': 0, + 'assembly.sol': 1, + 'ballot.sol': 0, + 'ballot_reentrant.sol': 0, + 'ballot_withoutWarnings.sol': 0, + 'cross_contract.sol': 0, + 'inheritance.sol': 0, + 'modifier1.sol': 0, + 'modifier2.sol': 0, + 'notReentrant.sol': 0, + 'structReentrant.sol': 0, + 'thisLocal.sol': 0 + } + + runModuleOnFiles(module, t, (file, report) => { + t.equal(report.length, lengthCheck[file], `${file} has right amount of tx.origin warnings`) + }) +}) + +test('Integration test gasCosts.js', function (t) { + t.plan(testFiles.length) + + var module = require('../../babelify-src/app/staticanalysis/modules/gasCosts') + + var lengthCheck = { + 'KingOfTheEtherThrone.sol': 2, + 'assembly.sol': 2, + 'ballot.sol': 3, + 'ballot_reentrant.sol': 2, + 'ballot_withoutWarnings.sol': 0, + 'cross_contract.sol': 1, + 'inheritance.sol': 1, + 'modifier1.sol': 0, + 'modifier2.sol': 1, + 'notReentrant.sol': 1, + 'structReentrant.sol': 1, + 'thisLocal.sol': 2 + } + + runModuleOnFiles(module, t, (file, report) => { + t.equal(report.length, lengthCheck[file], `${file} has right amount of gasCost warnings`) + }) +}) + +// #################### Helpers +function runModuleOnFiles (module, t, cb) { + var statRunner = new StatRunner() + + testFiles.forEach((fileName) => { + statRunner.runWithModuleList(testFileAsts[fileName], [{ name: module.name, mod: new module.Module() }], (reports) => { + cb(fileName, reports[0].report) + }) + }) +} diff --git a/test/staticanalysis/test-contracts/assembly.sol b/test/staticanalysis/test-contracts/assembly.sol index d1f63e75e1..a71c7e4a26 100644 --- a/test/staticanalysis/test-contracts/assembly.sol +++ b/test/staticanalysis/test-contracts/assembly.sol @@ -1,7 +1,9 @@ -pragma solidity ^0.4.9; + pragma solidity ^0.4.9; contract test { - function at(address _addr) returns (bytes o_code) { + address owner; + + function at(address _addr) returns (bytes o_code) { assembly { // retrieve the size of the code, this needs assembly let size := extcodesize(_addr) @@ -15,12 +17,13 @@ pragma solidity ^0.4.9; // actually retrieve the code, this needs assembly extcodecopy(_addr, add(o_code, 0x20), 0, size) } - } + } - function bla() { - msg.sender.send(19); + function bla() { + if(tx.origin == owner) + msg.sender.send(19); assembly { } - } + } }