From 954a1932f50fc2e948b550a753ea16f72acccef8 Mon Sep 17 00:00:00 2001 From: soad003 Date: Fri, 15 Dec 2017 17:01:32 +0100 Subject: [PATCH] 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')