From fd1c024de6d4f0f2104b3014d8c7f8a6596127d3 Mon Sep 17 00:00:00 2001 From: soad003 Date: Thu, 19 Apr 2018 17:33:37 +0200 Subject: [PATCH 1/2] Static Analysis: Bugfix constant function check, Similar var names allow number prefix, add delete dynamic array modul --- .../modules/checksEffectsInteraction.js | 6 +- .../src/analysis/modules/constantFunctions.js | 10 ++- .../analysis/modules/deleteDynamicArrays.js | 29 +++++++ .../src/analysis/modules/guardConditions.js | 2 +- remix-solidity/src/analysis/modules/list.js | 3 +- .../analysis/modules/similarVariableNames.js | 10 ++- .../analysis/modules/staticAnalysisCommon.js | 36 ++++++++- .../analysis/staticAnalysisCommon-test.js | 16 ++++ .../staticAnalysisIntegration-test.js | 77 +++++++++++++++---- .../test-contracts/deleteDynamicArray.sol | 37 +++++++++ 10 files changed, 200 insertions(+), 26 deletions(-) create mode 100644 remix-solidity/src/analysis/modules/deleteDynamicArrays.js create mode 100644 remix-solidity/test/analysis/test-contracts/deleteDynamicArray.sol diff --git a/remix-solidity/src/analysis/modules/checksEffectsInteraction.js b/remix-solidity/src/analysis/modules/checksEffectsInteraction.js index 1e7e81224a..54a1dd051b 100644 --- a/remix-solidity/src/analysis/modules/checksEffectsInteraction.js +++ b/remix-solidity/src/analysis/modules/checksEffectsInteraction.js @@ -33,10 +33,10 @@ function report (contracts, multipleContractsWithSameName) { contract.functions.forEach((func) => { if (isPotentialVulnerableFunction(func, getContext(callGraph, contract, func))) { var funcName = common.getFullQuallyfiedFuncDefinitionIdent(contract.node, func.node, func.parameters) - var comments = (hasModifiers) ? '
Note: Modifiers are currently not considered by this static analysis.' : '' - comments += (multipleContractsWithSameName) ? '
Note: Import aliases are currently not supported by this static analysis.' : '' + var comments = (hasModifiers) ? 'Note: Modifiers are currently not considered by this static analysis.' : '' + comments += (multipleContractsWithSameName) ? 'Note: Import aliases are currently not supported by this static analysis.' : '' warnings.push({ - warning: `Potential Violation of Checks-Effects-Interaction pattern in ${funcName}: Could potentially lead to re-entrancy vulnerability. ${comments}`, + warning: `Potential Violation of Checks-Effects-Interaction pattern in ${funcName}: Could potentially lead to re-entrancy vulnerability. ${comments}`, location: func.src, more: 'http://solidity.readthedocs.io/en/develop/security-considerations.html#re-entrancy' }) diff --git a/remix-solidity/src/analysis/modules/constantFunctions.js b/remix-solidity/src/analysis/modules/constantFunctions.js index 82750772ea..5da3562eb6 100644 --- a/remix-solidity/src/analysis/modules/constantFunctions.js +++ b/remix-solidity/src/analysis/modules/constantFunctions.js @@ -16,7 +16,8 @@ function constantFunctions () { common.isLocalCallGraphRelevantNode(node) || common.isInlineAssembly(node) || common.isNewExpression(node) || - common.isSelfdestructCall(node) + common.isSelfdestructCall(node) || + common.isDeleteUnaryOperation(node) ) this.report = this.abstractAst.build_report(report) @@ -45,8 +46,8 @@ function report (contracts, multipleContractsWithSameName) { contract.functions.filter((func) => common.hasFunctionBody(func.node)).forEach((func) => { if (common.isConstantFunction(func.node) !== func.potentiallyshouldBeConst) { var funcName = common.getFullQuallyfiedFuncDefinitionIdent(contract.node, func.node, func.parameters) - var comments = (hasModifiers) ? '
Note: Modifiers are currently not considered by this static analysis.' : '' - comments += (multipleContractsWithSameName) ? '
Note: Import aliases are currently not supported by this static analysis.' : '' + var comments = (hasModifiers) ? 'Note: Modifiers are currently not considered by this static analysis.' : '' + comments += (multipleContractsWithSameName) ? 'Note: Import aliases are currently not supported by this static analysis.' : '' if (func.potentiallyshouldBeConst) { warnings.push({ warning: `${funcName} : Potentially should be constant but is not. ${comments}`, @@ -87,7 +88,8 @@ function isConstBreaker (node, context) { common.isCallToNonConstLocalFunction(node) || common.isInlineAssembly(node) || common.isNewExpression(node) || - common.isSelfdestructCall(node) + common.isSelfdestructCall(node) || + common.isDeleteUnaryOperation(node) } function isCallOnNonConstExternalInterfaceFunction (node, context) { diff --git a/remix-solidity/src/analysis/modules/deleteDynamicArrays.js b/remix-solidity/src/analysis/modules/deleteDynamicArrays.js new file mode 100644 index 0000000000..b1143d179e --- /dev/null +++ b/remix-solidity/src/analysis/modules/deleteDynamicArrays.js @@ -0,0 +1,29 @@ +var name = 'Delete on dynamic Array: ' +var desc = 'Use require and appropriately' +var categories = require('./categories') +var common = require('./staticAnalysisCommon') + +function deleteDynamicArrays () { + this.rel = [] +} + +deleteDynamicArrays.prototype.visit = function (node) { + if (common.isDeleteOfDynamicArray(node)) this.rel.push(node) +} + +deleteDynamicArrays.prototype.report = function (compilationResults) { + return this.rel.map((node) => { + return { + warning: 'The “delete” operation when applied to a dynamically sized array in Solidity generates code to delete each of the elements contained. If the array is large, this operation can surpass the block gas limit and raise an OOG exception. Also nested dynamically sized objects can produce the same results.', + location: node.src, + more: 'http://solidity.readthedocs.io/en/latest/types.html?highlight=array#delete' + } + }) +} + +module.exports = { + name: name, + description: desc, + category: categories.GAS, + Module: deleteDynamicArrays +} diff --git a/remix-solidity/src/analysis/modules/guardConditions.js b/remix-solidity/src/analysis/modules/guardConditions.js index 7a4dbf548d..a9d877c57c 100644 --- a/remix-solidity/src/analysis/modules/guardConditions.js +++ b/remix-solidity/src/analysis/modules/guardConditions.js @@ -14,7 +14,7 @@ guardConditions.prototype.visit = function (node) { guardConditions.prototype.report = function (compilationResults) { if (this.guards.length > 0) { return [{ - warning: 'Use assert(x) if you never ever want x to be false, not in any circumstance (apart from a bug in your code). Use require(x) if x can be false, due to e.g. invalid input or a failing external component.', + warning: 'Use assert(x) if you never ever want x to be false, not in any circumstance (apart from a bug in your code). Use require(x) if x can be false, due to e.g. invalid input or a failing external component.', more: 'http://solidity.readthedocs.io/en/develop/control-structures.html#error-handling-assert-require-revert-and-exceptions' }] } diff --git a/remix-solidity/src/analysis/modules/list.js b/remix-solidity/src/analysis/modules/list.js index a4e0ea6b0e..3cbad9d87c 100644 --- a/remix-solidity/src/analysis/modules/list.js +++ b/remix-solidity/src/analysis/modules/list.js @@ -11,5 +11,6 @@ module.exports = [ require('./blockBlockhash'), require('./noReturn'), require('./selfdestruct'), - require('./guardConditions') + require('./guardConditions'), + require('./deleteDynamicArrays') ] diff --git a/remix-solidity/src/analysis/modules/similarVariableNames.js b/remix-solidity/src/analysis/modules/similarVariableNames.js index 1cbfcec9aa..ef4f5451a5 100644 --- a/remix-solidity/src/analysis/modules/similarVariableNames.js +++ b/remix-solidity/src/analysis/modules/similarVariableNames.js @@ -4,6 +4,8 @@ var categories = require('./categories') var common = require('./staticAnalysisCommon') var AbstractAst = require('./abstractAstView') var levenshtein = require('fast-levenshtein') +var remixLib = require('remix-lib') +var util = remixLib.util function similarVariableNames () { this.abstractAst = new AbstractAst() @@ -53,7 +55,7 @@ function findSimilarVarNames (vars) { var similar = [] var comb = {} vars.map((varName1) => vars.map((varName2) => { - if (varName1.length > 1 && varName2.length > 1 && varName2 !== varName1 && !isCommonPrefixedVersion(varName1, varName2) && !(comb[varName1 + ';' + varName2] || comb[varName2 + ';' + varName1])) { + if (varName1.length > 1 && varName2.length > 1 && varName2 !== varName1 && !isCommonPrefixedVersion(varName1, varName2) && !isCommonNrSuffixVersion(varName1, varName2) && !(comb[varName1 + ';' + varName2] || comb[varName2 + ';' + varName1])) { comb[varName1 + ';' + varName2] = true var distance = levenshtein.get(varName1, varName2) if (distance <= 2) similar.push({ var1: varName1, var2: varName2, distance: distance }) @@ -66,6 +68,12 @@ function isCommonPrefixedVersion (varName1, varName2) { return (varName1.startsWith('_') && varName1.slice(1) === varName2) || (varName2.startsWith('_') && varName2.slice(1) === varName1) } +function isCommonNrSuffixVersion (varName1, varName2) { + var ref = '^' + util.escapeRegExp(varName1.slice(0, -1)) + '[0-9]$' + + return varName2.match(ref) != null +} + function getFunctionVariables (contract, func) { return contract.stateVariables.concat(func.localVariables) } diff --git a/remix-solidity/src/analysis/modules/staticAnalysisCommon.js b/remix-solidity/src/analysis/modules/staticAnalysisCommon.js index e9fc7fce4c..9a041bdde5 100644 --- a/remix-solidity/src/analysis/modules/staticAnalysisCommon.js +++ b/remix-solidity/src/analysis/modules/staticAnalysisCommon.js @@ -58,7 +58,11 @@ var builtinFunctions = { 'selfdestruct(address)': true, 'revert()': true, 'assert(bool)': true, - 'require(bool)': true + 'require(bool)': true, + 'require(bool,string memory)': true, + 'revert(string memory)': true, + 'gasleft()': true, + 'blockhash(uint)': true } var lowLevelCallTypes = { @@ -404,6 +408,24 @@ function hasFunctionBody (funcNode) { return findFirstSubNodeLTR(funcNode, exactMatch(nodeTypes.BLOCK)) != null } +/** + * True if node is a delete instruction of a dynamic array + * @node {ASTNode} node to check for + * @return {bool} + */ +function isDeleteOfDynamicArray (node) { + return isDeleteUnaryOperation(node) && isDynamicArrayAccess(node.children[0]) +} + +/** + * True if node is node is a ref to a dynamic array + * @node {ASTNode} node to check for + * @return {bool} + */ +function isDynamicArrayAccess (node) { + return node && nodeType(node, exactMatch(nodeTypes.IDENTIFIER)) && (node.attributes.type.endsWith('[] storage ref') || node.attributes.type === 'bytes storage ref' || node.attributes.type === 'string storage ref') +} + /** * True if call to code within the current contracts context including (delegate) library call * @node {ASTNode} some AstNode @@ -540,6 +562,15 @@ function isPlusPlusUnaryOperation (node) { return nodeType(node, exactMatch(nodeTypes.UNARYOPERATION)) && operator(node, exactMatch(util.escapeRegExp('++'))) } +/** + * True if unary delete operation + * @node {ASTNode} some AstNode + * @return {bool} + */ +function isDeleteUnaryOperation (node) { + return nodeType(node, exactMatch(nodeTypes.UNARYOPERATION)) && operator(node, exactMatch(util.escapeRegExp('delete'))) +} + /** * True if unary decrement operation * @node {ASTNode} some AstNode @@ -827,6 +858,8 @@ module.exports = { getFunctionOrModifierDefinitionReturnParameterPart: getFunctionOrModifierDefinitionReturnParameterPart, // #################### Complex Node Identification + isDeleteOfDynamicArray: isDeleteOfDynamicArray, + isDynamicArrayAccess: isDynamicArrayAccess, hasFunctionBody: hasFunctionBody, isInteraction: isInteraction, isEffect: isEffect, @@ -858,6 +891,7 @@ module.exports = { isRequireCall: isRequireCall, // #################### Trivial Node Identification + isDeleteUnaryOperation: isDeleteUnaryOperation, isFunctionDefinition: isFunctionDefinition, isModifierDefinition: isModifierDefinition, isInheritanceSpecifier: isInheritanceSpecifier, diff --git a/remix-solidity/test/analysis/staticAnalysisCommon-test.js b/remix-solidity/test/analysis/staticAnalysisCommon-test.js index b9e39c81f2..fa740828ce 100644 --- a/remix-solidity/test/analysis/staticAnalysisCommon-test.js +++ b/remix-solidity/test/analysis/staticAnalysisCommon-test.js @@ -2094,3 +2094,19 @@ test('staticAnalysisCommon: function call with of function with function paramet t.equals(common.getFunctionCallTypeParameterType(node1), 'function (uint256,uint256) pure returns (uint256),uint256,uint256', 'Extracts param right type') }) + +test('staticAnalysisCommon: require call', function (t) { + t.plan(3) + var node = {'attributes': {'argumentTypes': null, 'isConstant': false, 'isLValue': false, 'isPure': false, 'isStructConstructorCall': false, 'lValueRequested': false, 'names': [null], 'type': 'tuple()', 'type_conversion': false}, 'children': [{'attributes': {'argumentTypes': [{'typeIdentifier': 't_bool', 'typeString': 'bool'}, {'typeIdentifier': 't_stringliteral_80efd193f332877914d93edb0b3ef5c6a7eecd00c6251c3fd7f146b60b40e6cd', 'typeString': 'literal_string \'fuu\''}], 'overloadedDeclarations': [90, 91], 'referencedDeclaration': 91, 'type': 'function (bool,string memory) pure', 'value': 'require'}, 'id': 50, 'name': 'Identifier', 'src': '462:7:0'}, {'attributes': {'argumentTypes': null, 'commonType': {'typeIdentifier': 't_address', 'typeString': 'address'}, 'isConstant': false, 'isLValue': false, 'isPure': false, 'lValueRequested': false, 'operator': '==', 'type': 'bool'}, 'children': [{'attributes': {'argumentTypes': null, 'isConstant': false, 'isLValue': false, 'isPure': false, 'lValueRequested': false, 'member_name': 'sender', 'referencedDeclaration': null, 'type': 'address'}, 'children': [{'attributes': {'argumentTypes': null, 'overloadedDeclarations': [null], 'referencedDeclaration': 87, 'type': 'msg', 'value': 'msg'}, 'id': 51, 'name': 'Identifier', 'src': '470:3:0'}], 'id': 52, 'name': 'MemberAccess', 'src': '470:10:0'}, {'attributes': {'argumentTypes': null, 'overloadedDeclarations': [null], 'referencedDeclaration': 10, 'type': 'address', 'value': 'owner'}, 'id': 53, 'name': 'Identifier', 'src': '484:5:0'}], 'id': 54, 'name': 'BinaryOperation', 'src': '470:19:0'}, {'attributes': {'argumentTypes': null, 'hexvalue': '667575', 'isConstant': false, 'isLValue': false, 'isPure': true, 'lValueRequested': false, 'subdenomination': null, 'token': 'string', 'type': 'literal_string \'fuu\'', 'value': 'fuu'}, 'id': 55, 'name': 'Literal', 'src': '491:5:0'}], 'id': 56, 'name': 'FunctionCall', 'src': '462:35:0'} + + t.equals(common.isRequireCall(node), true) + t.equals(common.getFunctionCallType(node), 'function (bool,string memory) pure', 'Extracts right type') + t.equals(common.getFunctionCallTypeParameterType(node), 'bool,string memory', 'Extracts param right type') +}) + +test('staticAnalysisCommon: isDeleteOfDynamicArray', function (t) { + t.plan(2) + var node = {'attributes': {'argumentTypes': null, 'isConstant': false, 'isLValue': false, 'isPure': false, 'lValueRequested': false, 'operator': 'delete', 'prefix': true, 'type': 'tuple()'}, 'children': [{'attributes': {'argumentTypes': null, 'overloadedDeclarations': [null], 'referencedDeclaration': 4, 'type': 'uint256[] storage ref', 'value': 'users'}, 'id': 58, 'name': 'Identifier', 'src': '514:5:0'}], 'id': 59, 'name': 'UnaryOperation', 'src': '507:12:0'} + t.equals(common.isDeleteOfDynamicArray(node), true) + t.equals(common.isDynamicArrayAccess(node.children[0]), true, 'Extracts right type') +}) diff --git a/remix-solidity/test/analysis/staticAnalysisIntegration-test.js b/remix-solidity/test/analysis/staticAnalysisIntegration-test.js index d708f6ea9b..6139c1e8a7 100644 --- a/remix-solidity/test/analysis/staticAnalysisIntegration-test.js +++ b/remix-solidity/test/analysis/staticAnalysisIntegration-test.js @@ -27,7 +27,8 @@ var testFiles = [ 'transfer.sol', 'ctor.sol', 'forgottenReturn.sol', - 'selfdestruct.sol' + 'selfdestruct.sol', + 'deleteDynamicArray.sol' ] var testFileAsts = {} @@ -60,7 +61,8 @@ test('Integration test thisLocal.js', function (t) { 'transfer.sol': 0, 'ctor.sol': 0, 'forgottenReturn.sol': 0, - 'selfdestruct.sol': 0 + 'selfdestruct.sol': 0, + 'deleteDynamicArray.sol': 0 } runModuleOnFiles(module, t, (file, report) => { @@ -91,7 +93,8 @@ test('Integration test checksEffectsInteraction.js', function (t) { 'transfer.sol': 1, 'ctor.sol': 0, 'forgottenReturn.sol': 0, - 'selfdestruct.sol': 0 + 'selfdestruct.sol': 0, + 'deleteDynamicArray.sol': 0 } runModuleOnFiles(module, t, (file, report) => { @@ -122,7 +125,8 @@ test('Integration test constantFunctions.js', function (t) { 'transfer.sol': 0, 'ctor.sol': 0, 'forgottenReturn.sol': 0, - 'selfdestruct.sol': 1 + 'selfdestruct.sol': 1, + 'deleteDynamicArray.sol': 0 } runModuleOnFiles(module, t, (file, report) => { @@ -153,7 +157,8 @@ test('Integration test inlineAssembly.js', function (t) { 'transfer.sol': 0, 'ctor.sol': 0, 'forgottenReturn.sol': 0, - 'selfdestruct.sol': 0 + 'selfdestruct.sol': 0, + 'deleteDynamicArray.sol': 0 } runModuleOnFiles(module, t, (file, report) => { @@ -184,7 +189,8 @@ test('Integration test txOrigin.js', function (t) { 'transfer.sol': 0, 'ctor.sol': 0, 'forgottenReturn.sol': 0, - 'selfdestruct.sol': 0 + 'selfdestruct.sol': 0, + 'deleteDynamicArray.sol': 0 } runModuleOnFiles(module, t, (file, report) => { @@ -215,7 +221,8 @@ test('Integration test gasCosts.js', function (t) { 'transfer.sol': 1, 'ctor.sol': 0, 'forgottenReturn.sol': 3, - 'selfdestruct.sol': 0 + 'selfdestruct.sol': 0, + 'deleteDynamicArray.sol': 2 } runModuleOnFiles(module, t, (file, report) => { @@ -246,7 +253,8 @@ test('Integration test similarVariableNames.js', function (t) { 'transfer.sol': 0, 'ctor.sol': 1, 'forgottenReturn.sol': 0, - 'selfdestruct.sol': 0 + 'selfdestruct.sol': 0, + 'deleteDynamicArray.sol': 1 } runModuleOnFiles(module, t, (file, report) => { @@ -277,7 +285,8 @@ test('Integration test inlineAssembly.js', function (t) { 'transfer.sol': 0, 'ctor.sol': 0, 'forgottenReturn.sol': 0, - 'selfdestruct.sol': 0 + 'selfdestruct.sol': 0, + 'deleteDynamicArray.sol': 0 } runModuleOnFiles(module, t, (file, report) => { @@ -308,7 +317,8 @@ test('Integration test blockTimestamp.js', function (t) { 'transfer.sol': 0, 'ctor.sol': 0, 'forgottenReturn.sol': 0, - 'selfdestruct.sol': 0 + 'selfdestruct.sol': 0, + 'deleteDynamicArray.sol': 0 } runModuleOnFiles(module, t, (file, report) => { @@ -339,7 +349,8 @@ test('Integration test lowLevelCalls.js', function (t) { 'transfer.sol': 0, 'ctor.sol': 0, 'forgottenReturn.sol': 0, - 'selfdestruct.sol': 0 + 'selfdestruct.sol': 0, + 'deleteDynamicArray.sol': 0 } runModuleOnFiles(module, t, (file, report) => { @@ -370,7 +381,8 @@ test('Integration test blockBlockhash.js', function (t) { 'transfer.sol': 0, 'ctor.sol': 0, 'forgottenReturn.sol': 0, - 'selfdestruct.sol': 0 + 'selfdestruct.sol': 0, + 'deleteDynamicArray.sol': 0 } runModuleOnFiles(module, t, (file, report) => { @@ -401,7 +413,8 @@ test('Integration test noReturn.js', function (t) { 'transfer.sol': 0, 'ctor.sol': 0, 'forgottenReturn.sol': 1, - 'selfdestruct.sol': 0 + 'selfdestruct.sol': 0, + 'deleteDynamicArray.sol': 0 } runModuleOnFiles(module, t, (file, report) => { @@ -432,7 +445,8 @@ test('Integration test selfdestruct.js', function (t) { 'transfer.sol': 0, 'ctor.sol': 0, 'forgottenReturn.sol': 0, - 'selfdestruct.sol': 2 + 'selfdestruct.sol': 2, + 'deleteDynamicArray.sol': 0 } runModuleOnFiles(module, t, (file, report) => { @@ -463,7 +477,8 @@ test('Integration test guardConditions.js', function (t) { 'transfer.sol': 0, 'ctor.sol': 0, 'forgottenReturn.sol': 0, - 'selfdestruct.sol': 0 + 'selfdestruct.sol': 0, + 'deleteDynamicArray.sol': 1 } runModuleOnFiles(module, t, (file, report) => { @@ -471,6 +486,38 @@ test('Integration test guardConditions.js', function (t) { }) }) +test('Integration test deleteDynamicArrays.js', function (t) { + t.plan(testFiles.length) + + var module = require('../../src/analysis/modules/deleteDynamicArrays') + + var lengthCheck = { + 'KingOfTheEtherThrone.sol': 0, + 'assembly.sol': 0, + '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, + 'globals.sol': 0, + 'library.sol': 0, + 'transfer.sol': 0, + 'ctor.sol': 0, + 'forgottenReturn.sol': 0, + 'selfdestruct.sol': 0, + 'deleteDynamicArray.sol': 2 + } + + runModuleOnFiles(module, t, (file, report) => { + t.equal(report.length, lengthCheck[file], `${file} has right amount of deleteDynamicArrays warnings`) + }) +}) + // #################### Helpers function runModuleOnFiles (module, t, cb) { var statRunner = new StatRunner() diff --git a/remix-solidity/test/analysis/test-contracts/deleteDynamicArray.sol b/remix-solidity/test/analysis/test-contracts/deleteDynamicArray.sol new file mode 100644 index 0000000000..bb179b21f6 --- /dev/null +++ b/remix-solidity/test/analysis/test-contracts/deleteDynamicArray.sol @@ -0,0 +1,37 @@ +pragma solidity ^0.4.22; +contract arr { + uint[] users; + + bytes access_rights_per_user; + + uint user_index; + + address owner; + + string grr = "message"; + + uint[100] last_100_users; + + constructor(address owner1) public { + owner = owner1; + user_index = 0; + } + + function addUser(uint id, byte rights) public{ + users[user_index] = id; + last_100_users[user_index % 100] = id; + access_rights_per_user[user_index] = rights; + user_index++; + } + + function resetState() public{ + require(msg.sender == owner, grr); + delete users; + delete access_rights_per_user; + delete last_100_users; + } + + function bla(string bal) public { + grr = bal; + } +} \ No newline at end of file From 94783ee1a22f7eed4cb705e9ef8986cf9267f49c Mon Sep 17 00:00:00 2001 From: soad003 Date: Fri, 20 Apr 2018 13:34:53 +0200 Subject: [PATCH 2/2] Static Analysis: fixes suggested by axic --- .../analysis/modules/similarVariableNames.js | 2 +- .../analysis/modules/staticAnalysisCommon.js | 47 +++++++++++++++++-- .../analysis/staticAnalysisCommon-test.js | 28 ++++++++++- 3 files changed, 70 insertions(+), 7 deletions(-) diff --git a/remix-solidity/src/analysis/modules/similarVariableNames.js b/remix-solidity/src/analysis/modules/similarVariableNames.js index ef4f5451a5..903fbab20d 100644 --- a/remix-solidity/src/analysis/modules/similarVariableNames.js +++ b/remix-solidity/src/analysis/modules/similarVariableNames.js @@ -69,7 +69,7 @@ function isCommonPrefixedVersion (varName1, varName2) { } function isCommonNrSuffixVersion (varName1, varName2) { - var ref = '^' + util.escapeRegExp(varName1.slice(0, -1)) + '[0-9]$' + var ref = '^' + util.escapeRegExp(varName1.slice(0, -1)) + '[0-9]*$' return varName2.match(ref) != null } diff --git a/remix-solidity/src/analysis/modules/staticAnalysisCommon.js b/remix-solidity/src/analysis/modules/staticAnalysisCommon.js index 9a041bdde5..8990bb03c4 100644 --- a/remix-solidity/src/analysis/modules/staticAnalysisCommon.js +++ b/remix-solidity/src/analysis/modules/staticAnalysisCommon.js @@ -27,7 +27,10 @@ var basicTypes = { UINT: 'uint256', BOOL: 'bool', ADDRESS: 'address', - BYTES32: 'bytes32' + BYTES32: 'bytes32', + STRING_MEM: 'string memory', + BYTES_MEM: 'bytes memory', + BYTES4: 'bytes4' } var basicRegex = { @@ -57,10 +60,10 @@ var builtinFunctions = { 'mulmod(uint256,uint256,uint256)': true, 'selfdestruct(address)': true, 'revert()': true, + 'revert(string memory)': true, 'assert(bool)': true, 'require(bool)': true, 'require(bool,string memory)': true, - 'revert(string memory)': true, 'gasleft()': true, 'blockhash(uint)': true } @@ -82,6 +85,29 @@ var specialVariables = { } } +var abiNamespace = { + ENCODE: { + obj: 'abi', + member: 'encode', + type: buildFunctionSignature([], [basicTypes.BYTES_MEM], false, 'pure') + }, + ENCODEPACKED: { + obj: 'abi', + member: 'encodePacked', + type: buildFunctionSignature([], [basicTypes.BYTES_MEM], false, 'pure') + }, + ENCODE_SELECT: { + obj: 'abi', + member: 'encodeWithSelector', + type: buildFunctionSignature([basicTypes.BYTES4], [basicTypes.BYTES_MEM], false, 'pure') + }, + ENCODE_SIG: { + obj: 'abi', + member: 'encodeWithSignature', + type: buildFunctionSignature([basicTypes.STRING_MEM], [basicTypes.BYTES_MEM], false, 'pure') + } +} + // #################### Trivial Getters function getType (node) { @@ -441,7 +467,16 @@ function isLocalCallGraphRelevantNode (node) { * @return {bool} */ function isBuiltinFunctionCall (node) { - return isLocalCall(node) && builtinFunctions[getLocalCallName(node) + '(' + getFunctionCallTypeParameterType(node) + ')'] === true + return (isLocalCall(node) && builtinFunctions[getLocalCallName(node) + '(' + getFunctionCallTypeParameterType(node) + ')'] === true) || isAbiNamespaceCall(node) +} + +/** + * True if is builtin function like assert, sha3, erecover, ... + * @node {ASTNode} some AstNode + * @return {bool} + */ +function isAbiNamespaceCall (node) { + return Object.keys(abiNamespace).some((key) => abiNamespace.hasOwnProperty(key) && node.children && node.children[0] && isSpecialVariableAccess(node.children[0], abiNamespace[key])) } /** @@ -809,8 +844,8 @@ function exactMatch (regexStr) { * list of return type names * @return {Boolean} isPayable */ -function buildFunctionSignature (paramTypes, returnTypes, isPayable) { - return 'function (' + util.concatWithSeperator(paramTypes, ',') + ')' + ((isPayable) ? ' payable' : '') + ((returnTypes.length) ? ' returns (' + util.concatWithSeperator(returnTypes, ',') + ')' : '') +function buildFunctionSignature (paramTypes, returnTypes, isPayable, additionalMods) { + return 'function (' + util.concatWithSeperator(paramTypes, ',') + ')' + ((isPayable) ? ' payable' : '') + ((additionalMods) ? ' ' + additionalMods : '') + ((returnTypes.length) ? ' returns (' + util.concatWithSeperator(returnTypes, ',') + ')' : '') } /** @@ -859,6 +894,8 @@ module.exports = { // #################### Complex Node Identification isDeleteOfDynamicArray: isDeleteOfDynamicArray, + isAbiNamespaceCall: isAbiNamespaceCall, + isSpecialVariableAccess: isSpecialVariableAccess, isDynamicArrayAccess: isDynamicArrayAccess, hasFunctionBody: hasFunctionBody, isInteraction: isInteraction, diff --git a/remix-solidity/test/analysis/staticAnalysisCommon-test.js b/remix-solidity/test/analysis/staticAnalysisCommon-test.js index fa740828ce..7d4fc4a95e 100644 --- a/remix-solidity/test/analysis/staticAnalysisCommon-test.js +++ b/remix-solidity/test/analysis/staticAnalysisCommon-test.js @@ -7,12 +7,20 @@ function escapeRegExp (str) { } test('staticAnalysisCommon.helpers.buildFunctionSignature', function (t) { - t.plan(7) + t.plan(9) t.equal(common.helpers.buildFunctionSignature([common.basicTypes.UINT, common.basicTypes.ADDRESS], [common.basicTypes.BOOL], false), 'function (uint256,address) returns (bool)', 'two params and return value without payable') + t.equal(common.helpers.buildFunctionSignature([common.basicTypes.UINT, common.basicTypes.ADDRESS], [common.basicTypes.BOOL], false, 'pure'), + 'function (uint256,address) pure returns (bool)', + 'two params and return value without payable but pure') + + t.equal(common.helpers.buildFunctionSignature([common.basicTypes.UINT, common.basicTypes.ADDRESS], [common.basicTypes.BOOL], true, 'pure'), + 'function (uint256,address) payable pure returns (bool)', + 'two params and return value without payable but pure') + t.equal(common.helpers.buildFunctionSignature([common.basicTypes.UINT, common.basicTypes.BYTES32, common.basicTypes.BYTES32], [], true), 'function (uint256,bytes32,bytes32) payable', 'three params and no return with payable') @@ -2110,3 +2118,21 @@ test('staticAnalysisCommon: isDeleteOfDynamicArray', function (t) { t.equals(common.isDeleteOfDynamicArray(node), true) t.equals(common.isDynamicArrayAccess(node.children[0]), true, 'Extracts right type') }) + +test('staticAnalysisCommon: isAbiNamespaceCall', function (t) { + t.plan(8) + var node1 = {'attributes': {'argumentTypes': null, 'isConstant': false, 'isLValue': false, 'isPure': false, 'isStructConstructorCall': false, 'lValueRequested': false, 'names': [null], 'type': 'bytes memory', 'type_conversion': false}, 'children': [{'attributes': {'argumentTypes': [{'typeIdentifier': 't_uint256', 'typeString': 'uint256'}, {'typeIdentifier': 't_uint256', 'typeString': 'uint256'}], 'isConstant': false, 'isLValue': false, 'isPure': false, 'lValueRequested': false, 'member_name': 'encode', 'referencedDeclaration': null, 'type': 'function () pure returns (bytes memory)'}, 'children': [{'attributes': {'argumentTypes': null, 'overloadedDeclarations': [null], 'referencedDeclaration': 64, 'type': 'abi', 'value': 'abi'}, 'id': 26, 'name': 'Identifier', 'src': '245: 3:0'}], 'id': 28, 'name': 'MemberAccess', 'src': '245:10:0'}, {'attributes': {'argumentTypes': null, 'overloadedDeclarations': [null], 'referencedDeclaration': 7, 'type': 'uint256', 'value': 'a'}, 'id': 29, 'name': 'Identifier', 'src': '256:1:0'}, {'attributes': {'argumentTypes': null, 'overloadedDeclarations': [null], 'referencedDeclaration': 15, 'type': 'uint256', 'value': 'b'}, 'id': 30, 'name': 'Identifier', 'src': '258:1:0'}], 'id': 31, 'name': 'FunctionCall', 'src': '245:15:0'} + var node2 = {'attributes': {'argumentTypes': null, 'isConstant': false, 'isLValue': false, 'isPure': false, 'isStructConstructorCall': false, 'lValueRequested': false, 'names': [null], 'type': 'bytes memory', 'type_conversion': false}, 'children': [{'attributes': {'argumentTypes': [{'typeIdentifier': 't_uint256', 'typeString': 'uint256'}, {'typeIdentifier': 't_uint256', 'typeString': 'uint256'}], 'isConstant': false, 'isLValue': false, 'isPure': false, 'lValueRequested': false, 'member_name': 'encodePacked', 'referencedDeclaration': null, 'type': 'function () pure returns (bytes memory)'}, 'children': [{'attributes': {'argumentTypes': null, 'overloadedDeclarations': [null], 'referencedDeclaration': 64, 'type': 'abi', 'value': 'abi'}, 'id': 33, 'name': 'Identifier', 'src': '279:3:0'}], 'id': 35, 'name': 'MemberAccess', 'src': '279:16:0'}, {'attributes': {'argumentTypes': null, 'overloadedDeclarations': [null], 'referencedDeclaration': 7, 'type': 'uint256', 'value': 'a'}, 'id': 36, 'name': 'Identifier', 'src': '296:1:0'}, {'attributes': {'argumentTypes': null, 'overloadedDeclarations': [null], 'referencedDeclaration': 15, 'type': 'uint256', 'value': 'b'}, 'id': 37, 'name': 'Identifier', 'src': '298:1:0'}], 'id': 38, 'name': 'FunctionCall', 'src': '279:21:0'} + var node3 = {'attributes': {'argumentTypes': null, 'isConstant': false, 'isLValue': false, 'isPure': false, 'isStructConstructorCall': false, 'lValueRequested': false, 'names': [null], 'type': 'bytes memory', 'type_conversion': false}, 'children': [{'attributes': {'argumentTypes': [{'typeIdentifier': 't_bytes4', 'typeString': 'bytes4'}, {'typeIdentifier': 't_uint256', 'typeString': 'uint256'}, {'typeIdentifier': 't_uint256', 'typeString': 'uint256'}], 'isConstant': false, 'isLValue': false, 'isPure': false, 'lValueRequested': false, 'member_name': 'encodeWithSelector', 'referencedDeclaration': null, 'type': 'function (bytes4) pure returns (bytes memory)'}, 'children': [{'attributes': {'argumentTypes': null, 'overloadedDeclarations': [null], 'referencedDeclaration': 64, 'type': 'abi', 'value': 'abi'}, 'id': 40, 'name': 'Identifier', 'src': '319:3:0'}], 'id': 42, 'name': 'MemberAccess', 'src': '319:22:0'}, {'attributes': {'argumentTypes': null, 'overloadedDeclarations': [null], 'referencedDeclaration': 19, 'type': 'bytes4', 'value': 'selector'}, 'id': 43, 'name': 'Identifier', 'src': '342:8:0'}, {'attributes': {'argumentTypes': null, 'overloadedDeclarations': [null], 'referencedDeclaration': 7, 'type': 'uint256', 'value': 'a'}, 'id': 44, 'name': 'Identifier', 'src': '352:1:0'}, {'attributes': {'argumentTypes': null, 'overloadedDeclarations': [null], 'referencedDeclaration': 15, 'type': 'uint256', 'value': 'b'}, 'id': 45, 'name': 'Identifier', 'src': '355:1:0'}], 'id': 46, 'name': 'FunctionCall', 'src': '319:38:0'} + var node4 = {'attributes': {'argumentTypes': null, 'isConstant': false, 'isLValue': false, 'isPure': false, 'isStructConstructorCall': false, 'lValueRequested': false, 'names': [null], 'type': 'bytes memory', 'type_conversion': false}, 'children': [{'attributes': {'argumentTypes': [{'typeIdentifier': 't_string_memory_ptr', 'typeString': 'string memory'}, {'typeIdentifier': 't_uint256', 'typeString': 'uint256'}, {'typeIdentifier': 't_uint256', 'typeString': 'uint256'}], 'isConstant': false, 'isLValue': false, 'isPure': false, 'lValueRequested': false, 'member_name': 'encodeWithSignature', 'referencedDeclaration': null, 'type': 'function (string memory) pure returns (bytes memory)'}, 'children': [{'attributes': {'argumentTypes': null, 'overloadedDeclarations': [null], 'referencedDeclaration': 64, 'type': 'abi', 'value': 'abi'}, 'id': 48, 'name': 'Identifier', 'src': '367:3:0'}], 'id': 50, 'name': 'MemberAccess', 'src': '367:23:0'}, {'attributes': {'argumentTypes': null, 'overloadedDeclarations': [null], 'referencedDeclaration': 11, 'type': 'string memory', 'value': 'sig'}, 'id': 51, 'name': 'Identifier', 'src': '391:3:0'}, {'attributes': {'argumentTypes': null, 'overloadedDeclarations': [null], 'referencedDeclaration': 7, 'type': 'uint256', 'value': 'a'}, 'id': 52, 'name': 'Identifier', 'src': '396:1:0'}, {'attributes': {'argumentTypes': null, 'overloadedDeclarations': [null], 'referencedDeclaration': 15, 'type': 'uint256', 'value': 'b'}, 'id': 53, 'name': 'Identifier', 'src': '399:1:0'}], 'id': 54, 'name': 'FunctionCall', 'src': '367:34:0'} + + t.equals(common.isAbiNamespaceCall(node1), true, 'encode abi') + t.equals(common.isAbiNamespaceCall(node2), true, 'encodePacked abi') + t.equals(common.isAbiNamespaceCall(node3), true, 'encodeWithSelector abi') + t.equals(common.isAbiNamespaceCall(node4), true, 'encodeWithSignature abi') + + t.equals(common.isBuiltinFunctionCall(node1), true, 'encode Builtin') + t.equals(common.isBuiltinFunctionCall(node2), true, 'encodePacked Builtin') + t.equals(common.isBuiltinFunctionCall(node3), true, 'encodeWithSelector Builtin') + t.equals(common.isBuiltinFunctionCall(node4), true, 'encodeWithSignature Builtin') +})