mirror
/
remix-project
mirror of https://github.com/ethereum/remix-project.git
2
0
Fork 0
Code Issues Releases Wiki Activity

Merge pull request #978 from ethereum/staticAnalysisModul

Browse Source 
Add Static analysis module
pull/5370/head
yann300 6 years ago committed by GitHub
parent d95a7e544d f9ab44cf42
commit 124a52d6fc
7 changed files with 270 additions and 22 deletions
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Show Stats Download Patch File Download Diff File
  1. 29
      remix-analyzer/src/solidity-analyzer/modules/deleteFromDynamicArray.js
  2. 33
      remix-analyzer/src/solidity-analyzer/modules/forLoopIteratesOverDynamicArray.js
  3. 4
      remix-analyzer/src/solidity-analyzer/modules/list.js
  4. 33
      remix-analyzer/src/solidity-analyzer/modules/staticAnalysisCommon.js
  5. 156
      remix-analyzer/test/analysis/staticAnalysisIntegration-test.js
  6. 22
      remix-analyzer/test/analysis/test-contracts/deleteFromDynamicArray.sol
  7. 15
      remix-analyzer/test/analysis/test-contracts/forLoopIteratesOverDynamicArray.sol

29
remix-analyzer/src/solidity-analyzer/modules/deleteFromDynamicArray.js
Unescape View File

@ -0,0 +1,29 @@
var name = 'Delete from dynamic Array: '
var desc = 'Using delete on an array leaves a gap'
var categories = require('./categories')
var common = require('./staticAnalysisCommon')
function deleteFromDynamicArray () {
this.relevantNodes = []
}
deleteFromDynamicArray.prototype.visit = function (node) {
if (common.isDeleteFromDynamicArray(node)) this.relevantNodes.push(node)
}
deleteFromDynamicArray.prototype.report = function (compilationResults) {
return this.relevantNodes.map((node) => {
return {
warning: 'Using delete on an array leaves a gap. The length of the array remains the same. If you want to remove the empty position you need to shift items manually and update the length property.\n',
location: node.src,
more: 'https://github.com/miguelmota/solidity-idiosyncrasies'
}
})
}
module.exports = {
name: name,
description: desc,
category: categories.MISC,
Module: deleteFromDynamicArray
}

33
remix-analyzer/src/solidity-analyzer/modules/forLoopIteratesOverDynamicArray.js
Unescape View File

@ -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
}

4
remix-analyzer/src/solidity-analyzer/modules/list.js
Unescape View File

@ -15,5 +15,7 @@ module.exports = [
require('./deleteDynamicArrays'),
require('./assignAndCompare'),
require('./erc20Decimals'),
require('./stringBytesLength')
require('./stringBytesLength'),
require('./deleteFromDynamicArray'),
require('./forLoopIteratesOverDynamicArray')
]

33
remix-analyzer/src/solidity-analyzer/modules/staticAnalysisCommon.js
Unescape View File

@ -71,7 +71,8 @@ var builtinFunctions = {
'require(bool)': true,
'require(bool,string memory)': true,
'gasleft()': true,
'blockhash(uint)': true
'blockhash(uint)': true,
'address(address)': true
}
var lowLevelCallTypes = {
@ -500,6 +501,24 @@ 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 node is a delete instruction for an element from a dynamic array
* @node {ASTNode} node to check for
* @return {bool}
*/
function isDeleteFromDynamicArray (node) {
return isDeleteUnaryOperation(node) && isIndexAccess(node.children[0])
}
/**
* True if node is the access of an index
* @node {ASTNode} node to check for
* @return {bool}
*/
function isIndexAccess (node) {
return node && node.name === 'IndexAccess'
}
/**
* True if call to code within the current contracts context including (delegate) library call
* @node {ASTNode} some AstNode
@ -886,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) {
@ -998,9 +1026,11 @@ module.exports = {
// #################### Complex Node Identification
isDeleteOfDynamicArray: isDeleteOfDynamicArray,
isDeleteFromDynamicArray: isDeleteFromDynamicArray,
isAbiNamespaceCall: isAbiNamespaceCall,
isSpecialVariableAccess: isSpecialVariableAccess,
isDynamicArrayAccess: isDynamicArrayAccess,
isIndexAccess: isIndexAccess,
isSubScopeWithTopLevelUnAssignedBinOp: isSubScopeWithTopLevelUnAssignedBinOp,
hasFunctionBody: hasFunctionBody,
isInteraction: isInteraction,
@ -1034,6 +1064,7 @@ module.exports = {
isIntDivision: isIntDivision,
isStringToBytesConversion: isStringToBytesConversion,
isBytesLengthCheck: isBytesLengthCheck,
isForLoop: isForLoop,
// #################### Trivial Node Identification
isDeleteUnaryOperation: isDeleteUnaryOperation,

156
remix-analyzer/test/analysis/staticAnalysisIntegration-test.js
Unescape View File

@ -29,10 +29,12 @@ var testFiles = [
'forgottenReturn.sol',
'selfdestruct.sol',
'deleteDynamicArray.sol',
'deleteFromDynamicArray.sol',
'blockLevelCompare.sol',
'intDivisionTruncate.sol',
'ERC20.sol',
'stringBytesLength.sol'
'stringBytesLength.sol',
'forLoopIteratesOverDynamicArray.sol'
]
var testFileAsts = {}
@ -67,10 +69,12 @@ test('Integration test thisLocal.js', function (t) {
'forgottenReturn.sol': 0,
'selfdestruct.sol': 0,
'deleteDynamicArray.sol': 0,
'deleteFromDynamicArray.sol': 0,
'blockLevelCompare.sol': 0,
'intDivisionTruncate.sol': 0,
'ERC20.sol': 0,
'stringBytesLength.sol': 0
'stringBytesLength.sol': 0,
'forLoopIteratesOverDynamicArray.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
@ -103,10 +107,12 @@ test('Integration test checksEffectsInteraction.js', function (t) {
'forgottenReturn.sol': 0,
'selfdestruct.sol': 0,
'deleteDynamicArray.sol': 0,
'deleteFromDynamicArray.sol': 0,
'blockLevelCompare.sol': 0,
'intDivisionTruncate.sol': 0,
'ERC20.sol': 0,
'stringBytesLength.sol': 0
'stringBytesLength.sol': 0,
'forLoopIteratesOverDynamicArray.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
@ -139,10 +145,12 @@ test('Integration test constantFunctions.js', function (t) {
'forgottenReturn.sol': 0,
'selfdestruct.sol': 1,
'deleteDynamicArray.sol': 0,
'deleteFromDynamicArray.sol': 0,
'blockLevelCompare.sol': 0,
'intDivisionTruncate.sol': 0,
'ERC20.sol': 0,
'stringBytesLength.sol': 0
'stringBytesLength.sol': 0,
'forLoopIteratesOverDynamicArray.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
@ -175,10 +183,12 @@ test('Integration test inlineAssembly.js', function (t) {
'forgottenReturn.sol': 0,
'selfdestruct.sol': 0,
'deleteDynamicArray.sol': 0,
'deleteFromDynamicArray.sol': 0,
'blockLevelCompare.sol': 0,
'intDivisionTruncate.sol': 0,
'ERC20.sol': 0,
'stringBytesLength.sol': 0
'stringBytesLength.sol': 0,
'forLoopIteratesOverDynamicArray.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
@ -211,10 +221,12 @@ test('Integration test txOrigin.js', function (t) {
'forgottenReturn.sol': 0,
'selfdestruct.sol': 0,
'deleteDynamicArray.sol': 0,
'deleteFromDynamicArray.sol': 0,
'blockLevelCompare.sol': 0,
'intDivisionTruncate.sol': 0,
'ERC20.sol': 0,
'stringBytesLength.sol': 0
'stringBytesLength.sol': 0,
'forLoopIteratesOverDynamicArray.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
@ -247,10 +259,12 @@ test('Integration test gasCosts.js', function (t) {
'forgottenReturn.sol': 3,
'selfdestruct.sol': 0,
'deleteDynamicArray.sol': 2,
'deleteFromDynamicArray.sol': 1,
'blockLevelCompare.sol': 1,
'intDivisionTruncate.sol': 1,
'ERC20.sol': 2,
'stringBytesLength.sol': 1
'stringBytesLength.sol': 1,
'forLoopIteratesOverDynamicArray.sol': 1
}
runModuleOnFiles(module, t, (file, report) => {
@ -283,10 +297,12 @@ test('Integration test similarVariableNames.js', function (t) {
'forgottenReturn.sol': 0,
'selfdestruct.sol': 0,
'deleteDynamicArray.sol': 1,
'deleteFromDynamicArray.sol': 0,
'blockLevelCompare.sol': 0,
'intDivisionTruncate.sol': 0,
'ERC20.sol': 0,
'stringBytesLength.sol': 0
'stringBytesLength.sol': 0,
'forLoopIteratesOverDynamicArray.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
@ -319,10 +335,12 @@ test('Integration test inlineAssembly.js', function (t) {
'forgottenReturn.sol': 0,
'selfdestruct.sol': 0,
'deleteDynamicArray.sol': 0,
'deleteFromDynamicArray.sol': 0,
'blockLevelCompare.sol': 0,
'intDivisionTruncate.sol': 0,
'ERC20.sol': 0,
'stringBytesLength.sol': 0
'stringBytesLength.sol': 0,
'forLoopIteratesOverDynamicArray.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
@ -355,10 +373,12 @@ test('Integration test blockTimestamp.js', function (t) {
'forgottenReturn.sol': 0,
'selfdestruct.sol': 0,
'deleteDynamicArray.sol': 0,
'deleteFromDynamicArray.sol': 0,
'blockLevelCompare.sol': 0,
'intDivisionTruncate.sol': 0,
'ERC20.sol': 0,
'stringBytesLength.sol': 0
'stringBytesLength.sol': 0,
'forLoopIteratesOverDynamicArray.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
@ -391,10 +411,12 @@ test('Integration test lowLevelCalls.js', function (t) {
'forgottenReturn.sol': 0,
'selfdestruct.sol': 0,
'deleteDynamicArray.sol': 0,
'deleteFromDynamicArray.sol': 0,
'blockLevelCompare.sol': 0,
'intDivisionTruncate.sol': 0,
'ERC20.sol': 0,
'stringBytesLength.sol': 0
'stringBytesLength.sol': 0,
'forLoopIteratesOverDynamicArray.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
@ -427,10 +449,12 @@ test('Integration test blockBlockhash.js', function (t) {
'forgottenReturn.sol': 0,
'selfdestruct.sol': 0,
'deleteDynamicArray.sol': 0,
'deleteFromDynamicArray.sol': 0,
'blockLevelCompare.sol': 0,
'intDivisionTruncate.sol': 0,
'ERC20.sol': 0,
'stringBytesLength.sol': 0
'stringBytesLength.sol': 0,
'forLoopIteratesOverDynamicArray.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
@ -463,10 +487,12 @@ test('Integration test noReturn.js', function (t) {
'forgottenReturn.sol': 1,
'selfdestruct.sol': 0,
'deleteDynamicArray.sol': 0,
'deleteFromDynamicArray.sol': 0,
'blockLevelCompare.sol': 0,
'intDivisionTruncate.sol': 0,
'ERC20.sol': 0,
'stringBytesLength.sol': 0
'stringBytesLength.sol': 0,
'forLoopIteratesOverDynamicArray.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
@ -499,10 +525,12 @@ test('Integration test selfdestruct.js', function (t) {
'forgottenReturn.sol': 0,
'selfdestruct.sol': 3,
'deleteDynamicArray.sol': 0,
'deleteFromDynamicArray.sol': 0,
'blockLevelCompare.sol': 0,
'ERC20.sol': 0,
'intDivisionTruncate.sol': 5,
'stringBytesLength.sol': 0
'stringBytesLength.sol': 0,
'forLoopIteratesOverDynamicArray.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
@ -535,10 +563,12 @@ test('Integration test guardConditions.js', function (t) {
'forgottenReturn.sol': 0,
'selfdestruct.sol': 0,
'deleteDynamicArray.sol': 1,
'deleteFromDynamicArray.sol': 0,
'blockLevelCompare.sol': 0,
'intDivisionTruncate.sol': 1,
'ERC20.sol': 0,
'stringBytesLength.sol': 0
'stringBytesLength.sol': 0,
'forLoopIteratesOverDynamicArray.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
@ -571,10 +601,12 @@ test('Integration test deleteDynamicArrays.js', function (t) {
'forgottenReturn.sol': 0,
'selfdestruct.sol': 0,
'deleteDynamicArray.sol': 2,
'deleteFromDynamicArray.sol': 0,
'blockLevelCompare.sol': 0,
'intDivisionTruncate.sol': 0,
'ERC20.sol': 0,
'stringBytesLength.sol': 0
'stringBytesLength.sol': 0,
'forLoopIteratesOverDynamicArray.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
@ -582,6 +614,44 @@ test('Integration test deleteDynamicArrays.js', function (t) {
})
})
test('Integration test deleteFromDynamicArray.js', function (t) {
t.plan(testFiles.length)
var module = require('../../src/solidity-analyzer/modules/deleteFromDynamicArray')
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': 0,
'deleteFromDynamicArray.sol': 1,
'blockLevelCompare.sol': 0,
'intDivisionTruncate.sol': 0,
'ERC20.sol': 0,
'stringBytesLength.sol': 0,
'forLoopIteratesOverDynamicArray.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
t.equal(report.length, lengthCheck[file], `${file} has right amount of deleteFromDynamicArray warnings`)
})
})
test('Integration test assignAndCompare.js', function (t) {
t.plan(testFiles.length)
@ -607,10 +677,12 @@ test('Integration test assignAndCompare.js', function (t) {
'forgottenReturn.sol': 0,
'selfdestruct.sol': 0,
'deleteDynamicArray.sol': 0,
'deleteFromDynamicArray.sol': 0,
'blockLevelCompare.sol': 8,
'intDivisionTruncate.sol': 0,
'ERC20.sol': 0,
'stringBytesLength.sol': 0
'stringBytesLength.sol': 0,
'forLoopIteratesOverDynamicArray.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
@ -643,10 +715,12 @@ test('Integration test intDivisionTruncate.js', function (t) {
'forgottenReturn.sol': 0,
'selfdestruct.sol': 0,
'deleteDynamicArray.sol': 0,
'deleteFromDynamicArray.sol': 0,
'blockLevelCompare.sol': 0,
'intDivisionTruncate.sol': 2,
'ERC20.sol': 0,
'stringBytesLength.sol': 0
'stringBytesLength.sol': 0,
'forLoopIteratesOverDynamicArray.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
@ -679,10 +753,12 @@ test('Integration test erc20Decimal.js', function (t) {
'forgottenReturn.sol': 0,
'selfdestruct.sol': 0,
'deleteDynamicArray.sol': 0,
'deleteFromDynamicArray.sol': 0,
'blockLevelCompare.sol': 0,
'intDivisionTruncate.sol': 0,
'ERC20.sol': 1,
'stringBytesLength.sol': 0
'stringBytesLength.sol': 0,
'forLoopIteratesOverDynamicArray.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
@ -715,10 +791,12 @@ test('Integration test stringBytesLength.js', function (t) {
'forgottenReturn.sol': 0,
'selfdestruct.sol': 0,
'deleteDynamicArray.sol': 0,
'deleteFromDynamicArray.sol': 0,
'blockLevelCompare.sol': 0,
'intDivisionTruncate.sol': 0,
'ERC20.sol': 0,
'stringBytesLength.sol': 1
'stringBytesLength.sol': 1,
'forLoopIteratesOverDynamicArray.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
@ -726,6 +804,44 @@ test('Integration test stringBytesLength.js', function (t) {
})
})
test('Integration test forLoopIteratesOverDynamicArray.js', function (t) {
t.plan(testFiles.length)
var module = require('../../src/solidity-analyzer/modules/forLoopIteratesOverDynamicArray')
var lengthCheck = {
'KingOfTheEtherThrone.sol': 0,
'assembly.sol': 0,
'ballot.sol': 2,
'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': 0,
'globals.sol': 0,
'library.sol': 0,
'transfer.sol': 0,
'ctor.sol': 0,
'forgottenReturn.sol': 0,
'selfdestruct.sol': 0,
'deleteDynamicArray.sol': 0,
'deleteFromDynamicArray.sol': 0,
'blockLevelCompare.sol': 0,
'intDivisionTruncate.sol': 0,
'ERC20.sol': 0,
'stringBytesLength.sol': 0,
'forLoopIteratesOverDynamicArray.sol': 1
}
runModuleOnFiles(module, t, (file, report) => {
t.equal(report.length, lengthCheck[file], `${file} has right amount of forLoopIteratesOverDynamicArray warnings`)
})
})
// #################### Helpers
function runModuleOnFiles (module, t, cb) {
var statRunner = new StatRunner()

22
remix-analyzer/test/analysis/test-contracts/deleteFromDynamicArray.sol
Unescape View File

@ -0,0 +1,22 @@
pragma solidity ^0.4.22;
contract arr {
uint[] array = [1,2,3];
function removeAtIndex() public returns (uint[]) {
delete array[1];
return array;
}
// TODO: deleteFromDynamicArray should not generate warnings if array item is shifted and removed
/* function safeRemoveAtIndex(uint index) returns (uint[]) {
if (index >= array.length) return;
for (uint i = index; i < array.length-1; i++) {
array[i] = array[i+1];
}
delete array[array.length-1];
array.length--;
return array;
} */
}

15
remix-analyzer/test/analysis/test-contracts/forLoopIteratesOverDynamicArray.sol
Unescape View File

@ -0,0 +1,15 @@
pragma solidity ^0.4.22;
contract forLoopArr {
uint[] array;
constructor(uint[] _array) {
array = _array;
}
function shiftArrItem(uint index) returns(uint[]) {
// TODO: for (uint i = index; i < array.length-1; i++) should also generate warning
for (uint i = index; i < array.length; i++) {
array[i] = array[i+1];
}
return array;
}
}
Write Preview
Loading…
Cancel
Save