diff --git a/remix-analyzer/src/solidity-analyzer/modules/list.js b/remix-analyzer/src/solidity-analyzer/modules/list.js index 91a8842fe5..6c7be2d630 100644 --- a/remix-analyzer/src/solidity-analyzer/modules/list.js +++ b/remix-analyzer/src/solidity-analyzer/modules/list.js @@ -16,5 +16,6 @@ module.exports = [ require('./assignAndCompare'), require('./erc20Decimals'), require('./stringBytesLength'), - require('./deleteFromDynamicArray') + require('./deleteFromDynamicArray'), + require('./forLoopIteratesOverDynamicArray') ] diff --git a/remix-analyzer/src/solidity-analyzer/modules/staticAnalysisCommon.js b/remix-analyzer/src/solidity-analyzer/modules/staticAnalysisCommon.js index 89cedfe536..eb5db00554 100644 --- a/remix-analyzer/src/solidity-analyzer/modules/staticAnalysisCommon.js +++ b/remix-analyzer/src/solidity-analyzer/modules/staticAnalysisCommon.js @@ -905,6 +905,15 @@ function isBytesLengthCheck (node) { return isMemberAccess(node, exactMatch(util.escapeRegExp(basicTypes.UINT)), undefined, util.escapeRegExp('bytes *'), 'length') } +/** + * True if it is a 'for' loop + * @node {ASTNode} some AstNode + * @return {bool} + */ +function isForLoop (node) { + return nodeType(node, exactMatch(nodeTypes.FORSTATEMENT)) +} + // #################### Complex Node Identification - Private function isMemberAccess (node, retType, accessor, accessorType, memberName) { @@ -1055,6 +1064,7 @@ module.exports = { isIntDivision: isIntDivision, isStringToBytesConversion: isStringToBytesConversion, isBytesLengthCheck: isBytesLengthCheck, + isForLoop: isForLoop, // #################### Trivial Node Identification isDeleteUnaryOperation: isDeleteUnaryOperation, diff --git a/remix-solidity/src/analysis/modules/forLoopIteratesOverDynamicArray.js b/remix-solidity/src/analysis/modules/forLoopIteratesOverDynamicArray.js new file mode 100644 index 0000000000..40b8c899a6 --- /dev/null +++ b/remix-solidity/src/analysis/modules/forLoopIteratesOverDynamicArray.js @@ -0,0 +1,33 @@ +var name = 'For loop iterates over dynamic array: ' +var desc = 'The number of \'for\' loop iterations depends on dynamic array\'s size' +var categories = require('./categories') +var common = require('./staticAnalysisCommon') + +function forLoopIteratesOverDynamicArray () { + this.relevantNodes = [] +} + +forLoopIteratesOverDynamicArray.prototype.visit = function (node) { + if (common.isForLoop(node) && + node.children[1].children[1].attributes.member_name === 'length' && + node.children[1].children[1].children[0].attributes.type.indexOf('[]') != -1) { + this.relevantNodes.push(node) + } +} + +forLoopIteratesOverDynamicArray.prototype.report = function (compilationResults) { + return this.relevantNodes.map((node) => { + return { + warning: 'Loops that do not have a fixed number of iterations, for example, loops that depend on storage values, have to be used carefully: Due to the block gas limit, transactions can only consume a certain amount of gas. The number of iterations in a loop can grow beyond the block gas limit which can cause the complete contract to be stalled at a certain point. Additionally, using unbounded loops incurs in a lot of avoidable gas costs. Carefully test how many items at maximum you can pass to such functions to make it successful.', + location: node.src, + more: 'http://solidity.readthedocs.io/en/v0.4.24/security-considerations.html#gas-limit-and-loops' + } + }) +} + +module.exports = { + name: name, + description: desc, + category: categories.GAS, + Module: forLoopIteratesOverDynamicArray +}