diff --git a/remix-analyzer/src/solidity-analyzer/modules/checksEffectsInteraction.ts b/remix-analyzer/src/solidity-analyzer/modules/checksEffectsInteraction.ts index 4e8c8344fb..832c5ef344 100644 --- a/remix-analyzer/src/solidity-analyzer/modules/checksEffectsInteraction.ts +++ b/remix-analyzer/src/solidity-analyzer/modules/checksEffectsInteraction.ts @@ -78,7 +78,7 @@ export default class checksEffectsInteraction implements AnalyzerModule { private isLocalCallWithStateChange (node: FunctionCallAstNode, context: Context): boolean { if (isLocalCallGraphRelevantNode(node)) { const func = resolveCallGraphSymbol(context.callGraph, getFullQualifiedFunctionCallIdent(context.currentContract.node, node)) - return !func || (func && func.node['changesState']) + return !func || (func && func['changesState']) } return false } diff --git a/remix-analyzer/src/solidity-analyzer/modules/staticAnalysisCommon.ts b/remix-analyzer/src/solidity-analyzer/modules/staticAnalysisCommon.ts index ce7f4b1658..2d4d6e40d7 100644 --- a/remix-analyzer/src/solidity-analyzer/modules/staticAnalysisCommon.ts +++ b/remix-analyzer/src/solidity-analyzer/modules/staticAnalysisCommon.ts @@ -169,6 +169,7 @@ function getFunctionCallType (func: FunctionCallAstNode): string { * @return {string} variable name written to */ function getEffectedVariableName (effectNode: AssignmentAstNode | UnaryOperationAstNode): string { + // console.log('getEffectedVariableName---effectNode---', effectNode) if (!isEffect(effectNode)) throw new Error('staticAnalysisCommon.js: not an effect Node') if(effectNode.nodeType === 'Assignment' || effectNode.nodeType === 'UnaryOperation') { const IdentNode = findFirstSubNodeLTR(effectNode, exactMatch(nodeTypes.IDENTIFIER)) @@ -623,7 +624,10 @@ function isStorageVariableDeclaration (node: VariableDeclarationAstNode): boolea * @return {bool} */ function isInteraction (node: FunctionCallAstNode): boolean { - return isLLCall(node.expression) || isLLSend(node.expression) || isExternalDirectCall(node) || isTransfer(node.expression) + // console.log('Inside isInteraction----------', node) + return isLLCall(node.expression) || isLLSend(node.expression) || isExternalDirectCall(node) || isTransfer(node.expression) || + // to cover case of address.call.value.gas , See: inheritance.sol + (node.expression && node.expression.expression && isLLCall(node.expression.expression)) } /** @@ -906,6 +910,9 @@ function isLLSend (node: MemberAccessAstNode): boolean { * @return {bool} */ function isLLCall (node: MemberAccessAstNode): boolean { + // if(node && node.nodeType === 'MemberAccess' && node.memberName !== 'call' && + // node.expression && node.expression.nodeType && nodeType(node.expression, exactMatch(nodeTypes.MEMBERACCESS))) + // node = node.expression; return isMemberAccess(node, exactMatch(util.escapeRegExp(lowLevelCallTypes.CALL.type)), undefined, exactMatch(basicTypes.ADDRESS), exactMatch(lowLevelCallTypes.CALL.ident)) || @@ -1008,6 +1015,7 @@ function isBytesLengthCheck (node: MemberAccessAstNode): boolean { function isMemberAccess (node: MemberAccessAstNode, retType: string, accessor: string| undefined, accessorType: string, memberName: string | undefined): boolean { if(node && nodeType(node, exactMatch('MemberAccess'))) { + // console.log('node inside memberaccess------', node) const nodeTypeDef: boolean = typeDescription(node, retType) // console.log('MemberAccess typeDef ->',nodeTypeDef) const nodeMemName: boolean = memName(node, memberName) diff --git a/remix-analyzer/test/analysis/staticAnalysisIntegration-test-0.5.0.ts b/remix-analyzer/test/analysis/staticAnalysisIntegration-test-0.5.0.ts index 8f77c2ae41..083f0279fb 100644 --- a/remix-analyzer/test/analysis/staticAnalysisIntegration-test-0.5.0.ts +++ b/remix-analyzer/test/analysis/staticAnalysisIntegration-test-0.5.0.ts @@ -14,9 +14,9 @@ const testFiles = [ 'assembly.sol', 'ballot.sol', 'ballot_reentrant.sol', - // 'ballot_withoutWarnings.sol', - // 'cross_contract.sol', - // 'inheritance.sol', + 'ballot_withoutWarnings.sol', + 'cross_contract.sol', + 'inheritance.sol', // 'modifier1.sol', // 'modifier2.sol', // 'notReentrant.sol', @@ -50,7 +50,7 @@ testFiles.forEach((fileName) => { test('Integration test thisLocal.js', function (t) { // console.log('testFileAsts---------',testFileAsts) // t.plan(testFiles.length) - t.plan(4) + t.plan(7) var module = require('../../dist/src/solidity-analyzer/modules/thisLocal').default @@ -59,9 +59,9 @@ test('Integration test thisLocal.js', function (t) { 'assembly.sol': 0, 'ballot.sol': 0, 'ballot_reentrant.sol': 1, - // 'ballot_withoutWarnings.sol': 0, - // 'cross_contract.sol': 0, - // 'inheritance.sol': 0, + 'ballot_withoutWarnings.sol': 0, + 'cross_contract.sol': 0, + 'inheritance.sol': 0, // 'modifier1.sol': 0, // 'modifier2.sol': 0, // 'notReentrant.sol': 0, @@ -89,7 +89,7 @@ test('Integration test thisLocal.js', function (t) { }) test('Integration test checksEffectsInteraction.js', function (t) { - t.plan(4) + t.plan(7) var module = require('../../dist/src/solidity-analyzer/modules/checksEffectsInteraction').default @@ -98,9 +98,9 @@ test('Integration test checksEffectsInteraction.js', function (t) { 'assembly.sol': 1, 'ballot.sol': 0, 'ballot_reentrant.sol': 1, - // 'ballot_withoutWarnings.sol': 0, - // 'cross_contract.sol': 0, - // 'inheritance.sol': 1, + 'ballot_withoutWarnings.sol': 0, + 'cross_contract.sol': 0, + 'inheritance.sol': 1, // 'modifier1.sol': 0, // 'modifier2.sol': 0, // 'notReentrant.sol': 0, @@ -128,7 +128,7 @@ test('Integration test checksEffectsInteraction.js', function (t) { }) test('Integration test constantFunctions.js', function (t) { - t.plan(4) + t.plan(7) var module = require('../../dist/src/solidity-analyzer/modules/constantFunctions').default @@ -137,9 +137,9 @@ test('Integration test constantFunctions.js', function (t) { 'assembly.sol': 0, 'ballot.sol': 0, 'ballot_reentrant.sol': 0, - // 'ballot_withoutWarnings.sol': 0, - // 'cross_contract.sol': 0, - // 'inheritance.sol': 0, + 'ballot_withoutWarnings.sol': 0, + 'cross_contract.sol': 0, + 'inheritance.sol': 0, // 'modifier1.sol': 1, // 'modifier2.sol': 0, // 'notReentrant.sol': 0, @@ -167,7 +167,7 @@ test('Integration test constantFunctions.js', function (t) { }) test('Integration test inlineAssembly.js', function (t) { - t.plan(4) + t.plan(7) var module = require('../../dist/src/solidity-analyzer/modules/inlineAssembly').default @@ -176,9 +176,9 @@ test('Integration test inlineAssembly.js', function (t) { 'assembly.sol': 2, 'ballot.sol': 0, 'ballot_reentrant.sol': 0, - // 'ballot_withoutWarnings.sol': 0, - // 'cross_contract.sol': 0, - // 'inheritance.sol': 0, + 'ballot_withoutWarnings.sol': 0, + 'cross_contract.sol': 0, + 'inheritance.sol': 0, // 'modifier1.sol': 0, // 'modifier2.sol': 0, // 'notReentrant.sol': 0, @@ -206,7 +206,7 @@ test('Integration test inlineAssembly.js', function (t) { }) test('Integration test txOrigin.js', function (t) { - t.plan(4) + t.plan(7) var module = require('../../dist/src/solidity-analyzer/modules/txOrigin').default @@ -215,9 +215,9 @@ test('Integration test txOrigin.js', function (t) { 'assembly.sol': 1, 'ballot.sol': 0, 'ballot_reentrant.sol': 0, - // 'ballot_withoutWarnings.sol': 0, - // 'cross_contract.sol': 0, - // 'inheritance.sol': 0, + 'ballot_withoutWarnings.sol': 0, + 'cross_contract.sol': 0, + 'inheritance.sol': 0, // 'modifier1.sol': 0, // 'modifier2.sol': 0, // 'notReentrant.sol': 0, @@ -245,7 +245,7 @@ test('Integration test txOrigin.js', function (t) { }) test('Integration test gasCosts.js', function (t) { - t.plan(4) + t.plan(7) var module = require('../../dist/src/solidity-analyzer/modules/gasCosts').default @@ -254,9 +254,9 @@ test('Integration test gasCosts.js', function (t) { 'assembly.sol': 2, 'ballot.sol': 3, 'ballot_reentrant.sol': 2, - // 'ballot_withoutWarnings.sol': 0, - // 'cross_contract.sol': 1, - // 'inheritance.sol': 1, + 'ballot_withoutWarnings.sol': 0, + 'cross_contract.sol': 1, + 'inheritance.sol': 1, // 'modifier1.sol': 0, // 'modifier2.sol': 1, // 'notReentrant.sol': 1, @@ -284,7 +284,7 @@ test('Integration test gasCosts.js', function (t) { }) test('Integration test similarVariableNames.js', function (t) { - t.plan(4) + t.plan(7) var module = require('../../dist/src/solidity-analyzer/modules/similarVariableNames').default @@ -293,9 +293,9 @@ test('Integration test similarVariableNames.js', function (t) { 'assembly.sol': 0, 'ballot.sol': 2, 'ballot_reentrant.sol': 11, - // 'ballot_withoutWarnings.sol': 0, - // 'cross_contract.sol': 0, - // 'inheritance.sol': 0, + 'ballot_withoutWarnings.sol': 0, + 'cross_contract.sol': 0, + 'inheritance.sol': 0, // 'modifier1.sol': 0, // 'modifier2.sol': 0, // 'notReentrant.sol': 1, @@ -323,7 +323,7 @@ test('Integration test similarVariableNames.js', function (t) { }) test('Integration test blockTimestamp.js', function (t) { - t.plan(4) + t.plan(7) var module = require('../../dist/src/solidity-analyzer/modules/blockTimestamp').default @@ -332,9 +332,9 @@ test('Integration test blockTimestamp.js', function (t) { 'assembly.sol': 0, 'ballot.sol': 0, 'ballot_reentrant.sol': 3, - // 'ballot_withoutWarnings.sol': 0, - // 'cross_contract.sol': 0, - // 'inheritance.sol': 0, + 'ballot_withoutWarnings.sol': 0, + 'cross_contract.sol': 0, + 'inheritance.sol': 0, // 'modifier1.sol': 0, // 'modifier2.sol': 0, // 'notReentrant.sol': 0, @@ -362,7 +362,7 @@ test('Integration test blockTimestamp.js', function (t) { }) test('Integration test lowLevelCalls.js', function (t) { - t.plan(4) + t.plan(7) var module = require('../../dist/src/solidity-analyzer/modules/lowLevelCalls').default @@ -371,9 +371,9 @@ test('Integration test lowLevelCalls.js', function (t) { 'assembly.sol': 1, 'ballot.sol': 0, 'ballot_reentrant.sol': 7, - // 'ballot_withoutWarnings.sol': 0, - // 'cross_contract.sol': 1, - // 'inheritance.sol': 1, + 'ballot_withoutWarnings.sol': 0, + 'cross_contract.sol': 1, + 'inheritance.sol': 1, // 'modifier1.sol': 0, // 'modifier2.sol': 0, // 'notReentrant.sol': 1, @@ -401,7 +401,7 @@ test('Integration test lowLevelCalls.js', function (t) { }) test('Integration test blockBlockhash.js', function (t) { - t.plan(4) + t.plan(7) var module = require('../../dist/src/solidity-analyzer/modules/blockBlockhash').default @@ -410,9 +410,9 @@ test('Integration test blockBlockhash.js', function (t) { 'assembly.sol': 0, 'ballot.sol': 0, 'ballot_reentrant.sol': 0, - // 'ballot_withoutWarnings.sol': 0, - // 'cross_contract.sol': 0, - // 'inheritance.sol': 0, + 'ballot_withoutWarnings.sol': 0, + 'cross_contract.sol': 0, + 'inheritance.sol': 0, // 'modifier1.sol': 0, // 'modifier2.sol': 0, // 'notReentrant.sol': 0, @@ -439,51 +439,8 @@ test('Integration test blockBlockhash.js', function (t) { }) }) -// /* - -// ! No return gives compilation error with solidity 0.5.0 - -// test('Integration test noReturn.js', function (t) { -// t.plan(testFiles.length) - -// var module = require('../../dist/src/solidity-analyzer/modules/noReturn') - -// var lengthCheck = { -// 'KingOfTheEtherThrone.sol': 0, -// 'assembly.sol': 1, -// 'ballot.sol': 0, -// 'ballot_reentrant.sol': 0, -// 'ballot_withoutWarnings.sol': 0, -// 'cross_contract.sol': 0, -// 'inheritance.sol': 0, -// 'modifier1.sol': 1, -// 'modifier2.sol': 0, -// 'notReentrant.sol': 0, -// 'structReentrant.sol': 0, -// 'thisLocal.sol': 1, -// 'globals.sol': 0, -// 'library.sol': 0, -// 'transfer.sol': 0, -// 'ctor.sol': 0, -// 'forgottenReturn.sol': 1, -// 'selfdestruct.sol': 0, -// 'deleteDynamicArray.sol': 0, -// 'deleteFromDynamicArray.sol': 0, -// '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 noReturn warnings`) -// }) -// }) -// */ - test('Integration test selfdestruct.js', function (t) { - t.plan(4) + t.plan(7) var module = require('../../dist/src/solidity-analyzer/modules/selfdestruct').default @@ -492,9 +449,9 @@ test('Integration test selfdestruct.js', function (t) { 'assembly.sol': 0, 'ballot.sol': 0, 'ballot_reentrant.sol': 0, - // 'ballot_withoutWarnings.sol': 0, - // 'cross_contract.sol': 0, - // 'inheritance.sol': 0, + 'ballot_withoutWarnings.sol': 0, + 'cross_contract.sol': 0, + 'inheritance.sol': 0, // 'modifier1.sol': 0, // 'modifier2.sol': 0, // 'notReentrant.sol': 0, @@ -522,7 +479,7 @@ test('Integration test selfdestruct.js', function (t) { }) test('Integration test guardConditions.js', function (t) { - t.plan(4) + t.plan(7) var module = require('../../dist/src/solidity-analyzer/modules/guardConditions').default @@ -531,9 +488,9 @@ test('Integration test guardConditions.js', function (t) { 'assembly.sol': 1, 'ballot.sol': 0, 'ballot_reentrant.sol': 0, - // 'ballot_withoutWarnings.sol': 0, - // 'cross_contract.sol': 0, - // 'inheritance.sol': 0, + 'ballot_withoutWarnings.sol': 0, + 'cross_contract.sol': 0, + 'inheritance.sol': 0, // 'modifier1.sol': 0, // 'modifier2.sol': 0, // 'notReentrant.sol': 0, @@ -561,7 +518,7 @@ test('Integration test guardConditions.js', function (t) { }) test('Integration test deleteDynamicArrays.js', function (t) { - t.plan(4) + t.plan(7) var module = require('../../dist/src/solidity-analyzer/modules/deleteDynamicArrays').default @@ -570,9 +527,9 @@ test('Integration test deleteDynamicArrays.js', function (t) { 'assembly.sol': 0, 'ballot.sol': 0, 'ballot_reentrant.sol': 0, - // 'ballot_withoutWarnings.sol': 0, - // 'cross_contract.sol': 0, - // 'inheritance.sol': 0, + 'ballot_withoutWarnings.sol': 0, + 'cross_contract.sol': 0, + 'inheritance.sol': 0, // 'modifier1.sol': 0, // 'modifier2.sol': 0, // 'notReentrant.sol': 0, @@ -600,7 +557,7 @@ test('Integration test deleteDynamicArrays.js', function (t) { }) test('Integration test deleteFromDynamicArray.js', function (t) { - t.plan(4) + t.plan(7) var module = require('../../dist/src/solidity-analyzer/modules/deleteFromDynamicArray').default @@ -609,9 +566,9 @@ test('Integration test deleteFromDynamicArray.js', function (t) { 'assembly.sol': 0, 'ballot.sol': 0, 'ballot_reentrant.sol': 0, - // 'ballot_withoutWarnings.sol': 0, - // 'cross_contract.sol': 0, - // 'inheritance.sol': 0, + 'ballot_withoutWarnings.sol': 0, + 'cross_contract.sol': 0, + 'inheritance.sol': 0, // 'modifier1.sol': 0, // 'modifier2.sol': 0, // 'notReentrant.sol': 0, @@ -639,7 +596,7 @@ test('Integration test deleteFromDynamicArray.js', function (t) { }) test('Integration test assignAndCompare.js', function (t) { - t.plan(4) + t.plan(7) var module = require('../../dist/src/solidity-analyzer/modules/assignAndCompare').default @@ -648,9 +605,9 @@ test('Integration test assignAndCompare.js', function (t) { 'assembly.sol': 0, 'ballot.sol': 0, 'ballot_reentrant.sol': 0, - // 'ballot_withoutWarnings.sol': 0, - // 'cross_contract.sol': 0, - // 'inheritance.sol': 0, + 'ballot_withoutWarnings.sol': 0, + 'cross_contract.sol': 0, + 'inheritance.sol': 0, // 'modifier1.sol': 0, // 'modifier2.sol': 0, // 'notReentrant.sol': 0, @@ -678,7 +635,7 @@ test('Integration test assignAndCompare.js', function (t) { }) test('Integration test intDivisionTruncate.js', function (t) { - t.plan(4) + t.plan(7) var module = require('../../dist/src/solidity-analyzer/modules/intDivisionTruncate').default @@ -687,9 +644,9 @@ test('Integration test intDivisionTruncate.js', function (t) { 'assembly.sol': 0, 'ballot.sol': 0, 'ballot_reentrant.sol': 0, - // 'ballot_withoutWarnings.sol': 0, - // 'cross_contract.sol': 0, - // 'inheritance.sol': 0, + 'ballot_withoutWarnings.sol': 0, + 'cross_contract.sol': 0, + 'inheritance.sol': 0, // 'modifier1.sol': 0, // 'modifier2.sol': 0, // 'notReentrant.sol': 0, @@ -717,7 +674,7 @@ test('Integration test intDivisionTruncate.js', function (t) { }) test('Integration test erc20Decimal.js', function (t) { - t.plan(4) + t.plan(7) var module = require('../../dist/src/solidity-analyzer/modules/erc20Decimals').default @@ -726,9 +683,9 @@ test('Integration test erc20Decimal.js', function (t) { 'assembly.sol': 0, 'ballot.sol': 0, 'ballot_reentrant.sol': 0, - // 'ballot_withoutWarnings.sol': 0, - // 'cross_contract.sol': 0, - // 'inheritance.sol': 0, + 'ballot_withoutWarnings.sol': 0, + 'cross_contract.sol': 0, + 'inheritance.sol': 0, // 'modifier1.sol': 0, // 'modifier2.sol': 0, // 'notReentrant.sol': 0, @@ -756,7 +713,7 @@ test('Integration test erc20Decimal.js', function (t) { }) test('Integration test stringBytesLength.js', function (t) { - t.plan(4) + t.plan(7) var module = require('../../dist/src/solidity-analyzer/modules/stringBytesLength').default @@ -765,9 +722,9 @@ test('Integration test stringBytesLength.js', function (t) { 'assembly.sol': 0, 'ballot.sol': 0, 'ballot_reentrant.sol': 0, - // 'ballot_withoutWarnings.sol': 0, - // 'cross_contract.sol': 0, - // 'inheritance.sol': 0, + 'ballot_withoutWarnings.sol': 0, + 'cross_contract.sol': 0, + 'inheritance.sol': 0, // 'modifier1.sol': 0, // 'modifier2.sol': 0, // 'notReentrant.sol': 0, @@ -795,7 +752,7 @@ test('Integration test stringBytesLength.js', function (t) { }) test('Integration test etherTransferInLoop.js', function (t) { - t.plan(4) + t.plan(7) var module = require('../../dist/src/solidity-analyzer/modules/etherTransferInLoop').default @@ -804,9 +761,9 @@ test('Integration test etherTransferInLoop.js', function (t) { 'assembly.sol': 0, 'ballot.sol': 0, 'ballot_reentrant.sol': 0, - // 'ballot_withoutWarnings.sol': 0, - // 'cross_contract.sol': 0, - // 'inheritance.sol': 0, + 'ballot_withoutWarnings.sol': 0, + 'cross_contract.sol': 0, + 'inheritance.sol': 0, // 'modifier1.sol': 0, // 'modifier2.sol': 0, // 'notReentrant.sol': 0, @@ -834,7 +791,7 @@ test('Integration test etherTransferInLoop.js', function (t) { }) test('Integration test forLoopIteratesOverDynamicArray.js', function (t) { - t.plan(4) + t.plan(7) var module = require('../../dist/src/solidity-analyzer/modules/forLoopIteratesOverDynamicArray').default @@ -843,9 +800,9 @@ test('Integration test forLoopIteratesOverDynamicArray.js', function (t) { 'assembly.sol': 0, 'ballot.sol': 2, 'ballot_reentrant.sol': 1, - // 'ballot_withoutWarnings.sol': 0, - // 'cross_contract.sol': 0, - // 'inheritance.sol': 0, + 'ballot_withoutWarnings.sol': 0, + 'cross_contract.sol': 0, + 'inheritance.sol': 0, // 'modifier1.sol': 0, // 'modifier2.sol': 0, // 'notReentrant.sol': 0,