From 954a1932f50fc2e948b550a753ea16f72acccef8 Mon Sep 17 00:00:00 2001 From: soad003 Date: Fri, 15 Dec 2017 17:01:32 +0100 Subject: [PATCH 1/2] Static Analysis: Fix bug when passing function as parameter --- .../analysis/modules/staticAnalysisCommon.js | 17 +- .../analysis/staticAnalysisCommon-test.js | 182 ++++++++++++++++++ .../analysis/staticAnalysisIssues-test.js | 35 ++++ .../test-contracts/functionParameters.sol | 16 ++ remix-solidity/test/tests.js | 11 +- 5 files changed, 255 insertions(+), 6 deletions(-) create mode 100644 remix-solidity/test/analysis/staticAnalysisIssues-test.js create mode 100644 remix-solidity/test/analysis/test-contracts/functionParameters.sol diff --git a/remix-solidity/src/analysis/modules/staticAnalysisCommon.js b/remix-solidity/src/analysis/modules/staticAnalysisCommon.js index 1cce5acf28..439a1fb849 100644 --- a/remix-solidity/src/analysis/modules/staticAnalysisCommon.js +++ b/remix-solidity/src/analysis/modules/staticAnalysisCommon.js @@ -278,7 +278,22 @@ function getFunctionOrModifierDefinitionReturnParameterPart (funcNode) { * @return {string} parameter signature */ function getFunctionCallTypeParameterType (func) { - return new RegExp(basicRegex.FUNCTIONSIGNATURE).exec(getFunctionCallType(func))[1] + var type = getFunctionCallType(func) + if (type.startsWith('function (')) { + var paramTypes = '' + var openPar = 1 + for (var x = 10; x < type.length; x++) { + var c = type.charAt(x) + if (c === '(') openPar++ + else if (c === ')') openPar-- + + if (openPar === 0) return paramTypes + + paramTypes += c + } + } else { + throw new Error('staticAnalysisCommon.js: cannot extract parameter types from function call') + } } /** diff --git a/remix-solidity/test/analysis/staticAnalysisCommon-test.js b/remix-solidity/test/analysis/staticAnalysisCommon-test.js index a059b1275f..b9e39c81f2 100644 --- a/remix-solidity/test/analysis/staticAnalysisCommon-test.js +++ b/remix-solidity/test/analysis/staticAnalysisCommon-test.js @@ -1912,3 +1912,185 @@ test('staticAnalysisCommon.isLowLevelCall', function (t) { t.notOk(common.isLowLevelCallcodeInst(callAst), 'call is not callcode') t.ok(common.isLowLevelDelegatecallInst(delegatecallAst) && common.isLowLevelCall(delegatecallAst), 'delegatecall is llc should work') }) + +test('staticAnalysisCommon: Call of parameter function', function (t) { + t.plan(7) + var node1 = { + 'attributes': { + 'argumentTypes': null, + 'isConstant': false, + 'isLValue': false, + 'isPure': false, + 'isStructConstructorCall': false, + 'lValueRequested': false, + 'names': [ + null + ], + 'type': 'uint256', + 'type_conversion': false + }, + 'children': [ + { + 'attributes': { + 'argumentTypes': [ + { + 'typeIdentifier': 't_uint256', + 'typeString': 'uint256' + }, + { + 'typeIdentifier': 't_uint256', + 'typeString': 'uint256' + } + ], + 'overloadedDeclarations': [ + null + ], + 'referencedDeclaration': 25, + 'type': 'function (uint256,uint256) pure returns (uint256)', + 'value': 'f' + }, + 'id': 34, + 'name': 'Identifier', + 'src': '267:1:0' + }, + { + 'attributes': { + 'argumentTypes': null, + 'overloadedDeclarations': [ + null + ], + 'referencedDeclaration': 27, + 'type': 'uint256', + 'value': 'x' + }, + 'id': 35, + 'name': 'Identifier', + 'src': '269:1:0' + }, + { + 'attributes': { + 'argumentTypes': null, + 'overloadedDeclarations': [ + null + ], + 'referencedDeclaration': 29, + 'type': 'uint256', + 'value': 'y' + }, + 'id': 36, + 'name': 'Identifier', + 'src': '272:1:0' + } + ], + 'id': 37, + 'name': 'FunctionCall', + 'src': '267:7:0' + } + + t.ok(common.isLocalCall(node1), 'is not LocalCall') + t.notOk(common.isThisLocalCall(node1), 'is not this local call') + t.notOk(common.isSuperLocalCall(node1), 'is not super local call') + t.notOk(common.isExternalDirectCall(node1), 'is not ExternalDirectCall') + t.notOk(common.isLibraryCall(node1), 'is not LibraryCall') + + t.equals(common.getFunctionCallType(node1), 'function (uint256,uint256) pure returns (uint256)', 'Extracts right type') + + t.equals(common.getFunctionCallTypeParameterType(node1), 'uint256,uint256', 'Extracts param right type') +}) + +test('staticAnalysisCommon: function call with of function with function parameter', function (t) { + t.plan(2) + var node1 = { + 'attributes': { + 'argumentTypes': null, + 'isConstant': false, + 'isLValue': false, + 'isPure': false, + 'isStructConstructorCall': false, + 'lValueRequested': false, + 'names': [ + null + ], + 'type': 'uint256', + 'type_conversion': false + }, + 'children': [ + { + 'attributes': { + 'argumentTypes': [ + { + 'typeIdentifier': 't_function_internal_pure$_t_uint256_$_t_uint256_$returns$_t_uint256_$', + 'typeString': 'function (uint256,uint256) pure returns (uint256)' + }, + { + 'typeIdentifier': 't_uint256', + 'typeString': 'uint256' + }, + { + 'typeIdentifier': 't_uint256', + 'typeString': 'uint256' + } + ], + 'overloadedDeclarations': [ + null + ], + 'referencedDeclaration': 40, + 'type': 'function (function (uint256,uint256) pure returns (uint256),uint256,uint256) pure returns (uint256)', + 'value': 'eval' + }, + 'id': 49, + 'name': 'Identifier', + 'src': '361:4:0' + }, + { + 'attributes': { + 'argumentTypes': null, + 'overloadedDeclarations': [ + null + ], + 'referencedDeclaration': 15, + 'type': 'function (uint256,uint256) pure returns (uint256)', + 'value': 'plus' + }, + 'id': 50, + 'name': 'Identifier', + 'src': '366:4:0' + }, + { + 'attributes': { + 'argumentTypes': null, + 'overloadedDeclarations': [ + null + ], + 'referencedDeclaration': 42, + 'type': 'uint256', + 'value': 'x' + }, + 'id': 51, + 'name': 'Identifier', + 'src': '372:1:0' + }, + { + 'attributes': { + 'argumentTypes': null, + 'overloadedDeclarations': [ + null + ], + 'referencedDeclaration': 44, + 'type': 'uint256', + 'value': 'y' + }, + 'id': 52, + 'name': 'Identifier', + 'src': '375:1:0' + } + ], + 'id': 53, + 'name': 'FunctionCall', + 'src': '361:16:0' + } + + t.equals(common.getFunctionCallType(node1), 'function (function (uint256,uint256) pure returns (uint256),uint256,uint256) pure returns (uint256)', 'Extracts right type') + + t.equals(common.getFunctionCallTypeParameterType(node1), 'function (uint256,uint256) pure returns (uint256),uint256,uint256', 'Extracts param right type') +}) diff --git a/remix-solidity/test/analysis/staticAnalysisIssues-test.js b/remix-solidity/test/analysis/staticAnalysisIssues-test.js new file mode 100644 index 0000000000..c79618d8b8 --- /dev/null +++ b/remix-solidity/test/analysis/staticAnalysisIssues-test.js @@ -0,0 +1,35 @@ +var test = require('tape') +var remixLib = require('remix-lib') + +var StatRunner = require('../../src/analysis/staticAnalysisRunner') +var compilerInput = remixLib.helpers.compiler.compilerInput + +var solc = require('solc/wrapper') +var compiler = solc(require('../../soljson')) + +var fs = require('fs') +var path = require('path') + +function compile (fileName) { + var content = fs.readFileSync(path.join(__dirname, 'test-contracts', fileName), 'utf8') + return JSON.parse(compiler.compileStandardWrapper(compilerInput(content))) +} + +test('staticAnalysisIssues.functionParameterPassingError', function (t) { + // https://github.com/ethereum/browser-solidity/issues/889#issuecomment-351746474 + t.plan(2) + var res = compile('functionParameters.sol') + + var module = require('../../src/analysis/modules/checksEffectsInteraction') + + var statRunner = new StatRunner() + + t.doesNotThrow(() => { + statRunner.runWithModuleList(res, [{ name: module.name, mod: new module.Module() }], (reports) => { + }) + }, true, 'Analysis should not throw') + + statRunner.runWithModuleList(res, [{ name: module.name, mod: new module.Module() }], (reports) => { + t.ok(!reports.some((mod) => mod.report.some((rep) => rep.warning.includes('INTERNAL ERROR')), 'Should not have internal errors')) + }) +}) diff --git a/remix-solidity/test/analysis/test-contracts/functionParameters.sol b/remix-solidity/test/analysis/test-contracts/functionParameters.sol new file mode 100644 index 0000000000..f1a9f4a0f0 --- /dev/null +++ b/remix-solidity/test/analysis/test-contracts/functionParameters.sol @@ -0,0 +1,16 @@ +pragma solidity ^0.4.18; + +contract B { + function plus(uint a, uint b) pure internal returns (uint) { + return a + b; + } + + function eval(function (uint, uint) pure internal returns (uint) f, uint x, uint y) pure internal returns (uint) { + return f(x, y); + } + + function calc(uint x, uint y) pure public returns (uint) { + return eval(plus, x, y); + // return plus(x, y); + } +} \ No newline at end of file diff --git a/remix-solidity/test/tests.js b/remix-solidity/test/tests.js index 567441f311..0a1eb061c0 100644 --- a/remix-solidity/test/tests.js +++ b/remix-solidity/test/tests.js @@ -1,7 +1,8 @@ -// require('./decoder/decodeInfo.js') -// require('./decoder/storageLocation.js') +require('./decoder/decodeInfo.js') +require('./decoder/storageLocation.js') require('./decoder/storageDecoder.js') -// require('./decoder/localDecoder.js') +require('./decoder/localDecoder.js') -// require('./analysis/staticAnalysisCommon-test.js') -// require('./analysis/staticAnalysisIntegration-test.js') +require('./analysis/staticAnalysisCommon-test.js') +require('./analysis/staticAnalysisIntegration-test.js') +require('./analysis/staticAnalysisIssues-test.js') From 9d2c032516cff51100663600bca8874c779386c1 Mon Sep 17 00:00:00 2001 From: yann300 Date: Wed, 3 Jan 2018 14:25:14 +0100 Subject: [PATCH 2/2] add tests --- remix-solidity/test/decoder/localDecoder.js | 4 +++- remix-solidity/test/decoder/stateTests/mapping.js | 9 ++++----- remix-solidity/test/decoder/vmCall.js | 3 +-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/remix-solidity/test/decoder/localDecoder.js b/remix-solidity/test/decoder/localDecoder.js index a3188b365e..53d5006a46 100644 --- a/remix-solidity/test/decoder/localDecoder.js +++ b/remix-solidity/test/decoder/localDecoder.js @@ -32,7 +32,9 @@ function test (st, vm, privateKey) { misc2LocalTest(st, vm, privateKey, output.contracts['test.sol']['miscLocal2'].evm.bytecode.object, output, function () { output = compiler.compileStandardWrapper(compilerInput(structArrayLocal.contract)) output = JSON.parse(output) - structArrayLocalTest(st, vm, privateKey, output.contracts['test.sol']['structArrayLocal'].evm.bytecode.object, output, function () {}) + structArrayLocalTest(st, vm, privateKey, output.contracts['test.sol']['structArrayLocal'].evm.bytecode.object, output, function () { + st.end() + }) }) }) }) diff --git a/remix-solidity/test/decoder/stateTests/mapping.js b/remix-solidity/test/decoder/stateTests/mapping.js index 3aac8010c5..15c654b6bf 100644 --- a/remix-solidity/test/decoder/stateTests/mapping.js +++ b/remix-solidity/test/decoder/stateTests/mapping.js @@ -17,12 +17,10 @@ module.exports = function testMappingStorage (st, cb) { } else { remixLib.global.web3.eth.getTransaction(txHash, (error, tx) => { if (error) { + console.log(error) st.end(error) } else { - testMapping(st, vm, privateKey, tx.contractAddress, output, (error) => { - st.end(error) - cb() - }) + testMapping(st, vm, privateKey, tx.contractAddress, output, cb) } }) } @@ -39,6 +37,7 @@ function testMapping (st, vm, privateKey, contractAddress, output, cb) { console.log(txHash) remixLib.global.web3.eth.getTransaction(txHash, (error, tx) => { if (error) { + console.log(error) st.end(error) } else { var TraceManager = require('remix-core').trace.TraceManager @@ -62,7 +61,7 @@ function testMapping (st, vm, privateKey, contractAddress, output, cb) { st.equal(result['_iBreakSolidityStateInt'].type, 'mapping(uint256 => uint256)') st.equal(result['_iBreakSolidityStateInt'].value['0000000000000000000000000000000000000000000000000000000000000001'].value, '1') st.equal(result['_iBreakSolidityStateInt'].value['0000000000000000000000000000000000000000000000000000000000000001'].type, 'uint256') - // st.end() + cb() }, (reason) => { console.log('fail') st.end(reason) diff --git a/remix-solidity/test/decoder/vmCall.js b/remix-solidity/test/decoder/vmCall.js index 0faf7a483a..4aa6f40b8b 100644 --- a/remix-solidity/test/decoder/vmCall.js +++ b/remix-solidity/test/decoder/vmCall.js @@ -3,6 +3,7 @@ var utileth = require('ethereumjs-util') var Tx = require('ethereumjs-tx') var Block = require('ethereumjs-block') var BN = require('ethereumjs-util').BN +var remixLib = require('remix-lib') function sendTx (vm, from, to, value, data, cb) { var tx = new Tx({ @@ -33,7 +34,6 @@ function sendTx (vm, from, to, value, data, cb) { Init VM / Send Transaction */ function initVM (st, privateKey) { - var remixLib = require('remix-lib') var utileth = require('ethereumjs-util') var VM = require('ethereumjs-vm') var Web3Providers = remixLib.vm.Web3Providers @@ -52,7 +52,6 @@ function initVM (st, privateKey) { st.fail(mes) } else { remixLib.global.web3 = obj - st.end() } }) return vm