From 8fcfb419756c8465deccc604cc5c6b6756c611e5 Mon Sep 17 00:00:00 2001 From: yann300 Date: Mon, 23 Oct 2017 16:25:59 +0200 Subject: [PATCH] staticAnaysis switch warning to be a dom node --- .../staticanalysis/modules/blockBlockhash.js | 5 +++-- .../staticanalysis/modules/blockTimestamp.js | 9 +++++---- .../modules/checksEffectsInteraction.js | 3 ++- .../staticanalysis/modules/constantFunctions.js | 5 +++-- src/app/staticanalysis/modules/gasCosts.js | 9 +++++---- .../staticanalysis/modules/inlineAssembly.js | 5 +++-- src/app/staticanalysis/modules/lowLevelCalls.js | 17 +++++++++-------- .../modules/similarVariableNames.js | 14 +++++++++++--- src/app/staticanalysis/modules/thisLocal.js | 3 ++- src/app/staticanalysis/modules/txOrigin.js | 5 +++-- src/app/staticanalysis/staticAnalysisView.js | 5 +++-- 11 files changed, 49 insertions(+), 31 deletions(-) diff --git a/src/app/staticanalysis/modules/blockBlockhash.js b/src/app/staticanalysis/modules/blockBlockhash.js index 094c2d33d4..17f761ae3e 100644 --- a/src/app/staticanalysis/modules/blockBlockhash.js +++ b/src/app/staticanalysis/modules/blockBlockhash.js @@ -2,6 +2,7 @@ var name = 'Block.blockhash usage: ' var desc = 'Semantics maybe unclear' var categories = require('./categories') var common = require('./staticAnalysisCommon') +var yo = require('yo-yo') function blockBlockhash () { this.warningNodes = [] @@ -14,10 +15,10 @@ blockBlockhash.prototype.visit = function (node) { blockBlockhash.prototype.report = function (compilationResults) { return this.warningNodes.map(function (item, i) { return { - warning: `use of "block.blockhash": "block.blockhash" is used to access the last 256 block hashes. + warning: yo`use of "block.blockhash": "block.blockhash" is used to access the last 256 block hashes. A miner computes the block hash by "summing up" the information in the current block mined. By "summing up" the information in a clever way a miner can try to influence the outcome of a transaction in the current block. - This is especially easy if there are only a small number of equally likely outcomes.`, + This is especially easy if there are only a small number of equally likely outcomes.`, location: item.src } }) diff --git a/src/app/staticanalysis/modules/blockTimestamp.js b/src/app/staticanalysis/modules/blockTimestamp.js index a91d77ff81..0c542509d0 100644 --- a/src/app/staticanalysis/modules/blockTimestamp.js +++ b/src/app/staticanalysis/modules/blockTimestamp.js @@ -2,6 +2,7 @@ var name = 'Block timestamp: ' var desc = 'Semantics maybe unclear' var categories = require('./categories') var common = require('./staticAnalysisCommon') +var yo = require('yo-yo') function blockTimestamp () { this.warningNowNodes = [] @@ -16,15 +17,15 @@ blockTimestamp.prototype.visit = function (node) { blockTimestamp.prototype.report = function (compilationResults) { return this.warningNowNodes.map(function (item, i) { return { - warning: `use of "now": "now" does not mean current time. Now is an alias for block.timestamp. - Block.timestamp can be influenced by miners to a certain degree, be careful.`, + warning: yo`use of "now": "now" does not mean current time. Now is an alias for block.timestamp. + Block.timestamp can be influenced by miners to a certain degree, be careful.`, location: item.src, more: 'http://solidity.readthedocs.io/en/develop/frequently-asked-questions.html#are-timestamps-now-block-timestamp-reliable' } }).concat(this.warningblockTimestampNodes.map(function (item, i) { return { - warning: `use of "block.timestamp": "block.timestamp" can be influenced by miners to a certain degree. - That means that a miner can "choose" the block.timestamp, to a certain degree, to change the outcome of a transaction in the mined block.`, + warning: yo`use of "block.timestamp": "block.timestamp" can be influenced by miners to a certain degree. + That means that a miner can "choose" the block.timestamp, to a certain degree, to change the outcome of a transaction in the mined block.`, location: item.src, more: 'http://solidity.readthedocs.io/en/develop/frequently-asked-questions.html#are-timestamps-now-block-timestamp-reliable' } diff --git a/src/app/staticanalysis/modules/checksEffectsInteraction.js b/src/app/staticanalysis/modules/checksEffectsInteraction.js index 1e7e81224a..4d0f113351 100644 --- a/src/app/staticanalysis/modules/checksEffectsInteraction.js +++ b/src/app/staticanalysis/modules/checksEffectsInteraction.js @@ -4,6 +4,7 @@ var categories = require('./categories') var common = require('./staticAnalysisCommon') var fcallGraph = require('./functionCallGraph') var AbstractAst = require('./abstractAstView') +var yo = require('yo-yo') function checksEffectsInteraction () { this.abstractAst = new AbstractAst() @@ -36,7 +37,7 @@ function report (contracts, multipleContractsWithSameName) { 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: yo`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/src/app/staticanalysis/modules/constantFunctions.js b/src/app/staticanalysis/modules/constantFunctions.js index eed3578a47..23e7714ba9 100644 --- a/src/app/staticanalysis/modules/constantFunctions.js +++ b/src/app/staticanalysis/modules/constantFunctions.js @@ -4,6 +4,7 @@ var categories = require('./categories') var common = require('./staticAnalysisCommon') var fcallGraph = require('./functionCallGraph') var AbstractAst = require('./abstractAstView') +var yo = require('yo-yo') function constantFunctions () { this.abstractAst = new AbstractAst() @@ -38,13 +39,13 @@ function report (contracts, multipleContractsWithSameName) { 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}`, + warning: yo`${funcName}: Potentially should be constant but is not. ${comments}`, location: func.src, more: 'http://solidity.readthedocs.io/en/develop/contracts.html#constant-functions' }) } else { warnings.push({ - warning: `${funcName}: Is constant but potentially should not be. ${comments}`, + warning: yo`${funcName}: Is constant but potentially should not be. ${comments}`, location: func.src, more: 'http://solidity.readthedocs.io/en/develop/contracts.html#constant-functions' }) diff --git a/src/app/staticanalysis/modules/gasCosts.js b/src/app/staticanalysis/modules/gasCosts.js index 60675e6e03..4c85234529 100644 --- a/src/app/staticanalysis/modules/gasCosts.js +++ b/src/app/staticanalysis/modules/gasCosts.js @@ -1,6 +1,7 @@ var name = 'Gas costs: ' var desc = 'Warn if the gas requirements of functions are too high.' var categories = require('./categories') +var yo = require('yo-yo') function gasCosts () { } @@ -19,8 +20,8 @@ gasCosts.prototype.report = function (compilationResults) { if (fallback !== undefined) { if (fallback === null || fallback >= 2100) { report.push({ - warning: `Fallback function of contract ${contractName} requires too much gas (${fallback}).
- If the fallback function requires more than 2300 gas, the contract cannot receive Ether.` + warning: yo`Fallback function of contract ${contractName} requires too much gas (${fallback}).
+ If the fallback function requires more than 2300 gas, the contract cannot receive Ether.` }) } } @@ -33,10 +34,10 @@ gasCosts.prototype.report = function (compilationResults) { var gasString = gas === null ? 'unknown or not constant' : 'high: ' + gas if (gas === null || gas >= 3000000) { report.push({ - warning: `Gas requirement of function ${contractName}.${functionName} ${gasString}.
+ warning: yo`Gas requirement of function ${contractName}.${functionName} ${gasString}.
If the gas requirement of a function is higher than the block gas limit, it cannot be executed. Please avoid loops in your functions or actions that modify large areas of storage - (this includes clearing or copying arrays in storage)` + (this includes clearing or copying arrays in storage)` }) } } diff --git a/src/app/staticanalysis/modules/inlineAssembly.js b/src/app/staticanalysis/modules/inlineAssembly.js index ab9b6e1d03..44eef006f6 100644 --- a/src/app/staticanalysis/modules/inlineAssembly.js +++ b/src/app/staticanalysis/modules/inlineAssembly.js @@ -2,6 +2,7 @@ var name = 'Inline assembly: ' var desc = 'Use of Inline Assembly' var categories = require('./categories') var common = require('./staticAnalysisCommon') +var yo = require('yo-yo') function inlineAssembly () { this.inlineAssNodes = [] @@ -14,8 +15,8 @@ inlineAssembly.prototype.visit = function (node) { inlineAssembly.prototype.report = function (compilationResults) { return this.inlineAssNodes.map((node) => { return { - warning: `CAUTION: The Contract uses inline assembly, this is only advised in rare cases. - Additionally static analysis modules do not parse inline Assembly, this can lead to wrong analysis results.`, + warning: yo`CAUTION: The Contract uses inline assembly, this is only advised in rare cases. + Additionally static analysis modules do not parse inline Assembly, this can lead to wrong analysis results.`, location: node.src, more: 'http://solidity.readthedocs.io/en/develop/assembly.html#solidity-assembly' } diff --git a/src/app/staticanalysis/modules/lowLevelCalls.js b/src/app/staticanalysis/modules/lowLevelCalls.js index c56aa81e6c..3910156070 100644 --- a/src/app/staticanalysis/modules/lowLevelCalls.js +++ b/src/app/staticanalysis/modules/lowLevelCalls.js @@ -2,6 +2,7 @@ var name = 'Low level calls: ' var desc = 'Semantics maybe unclear' var categories = require('./categories') var common = require('./staticAnalysisCommon') +var yo = require('yo-yo') function lowLevelCalls () { this.llcNodes = [] @@ -25,29 +26,29 @@ lowLevelCalls.prototype.report = function (compilationResults) { var morehref = null switch (item.type) { case common.lowLevelCallTypes.CALL: - text = `use of "call": the use of low level "call" should be avoided whenever possible. + text = yo`use of "call": the use of low level "call" should be avoided whenever possible. It can lead to unexpected behavior if return value is not handled properly. - Please use Direct Calls via specifying the called contract's interface.
` + Please use Direct Calls via specifying the called contract's interface.
` morehref = 'http://solidity.readthedocs.io/en/develop/control-structures.html?#external-function-calls' // http://solidity.readthedocs.io/en/develop/frequently-asked-questions.html?#why-is-the-low-level-function-call-less-favorable-than-instantiating-a-contract-with-a-variable-contractb-b-and-executing-its-functions-b-dosomething break case common.lowLevelCallTypes.CALLCODE: - text = `use of "callcode": the use of low level "callcode" should be avoided whenever possible. + text = yo`use of "callcode": the use of low level "callcode" should be avoided whenever possible. External code that is called can change the state of the calling contract and send ether form the caller's balance. - If this is wantend behaviour use the Solidity library feature if possible.
` + If this is wantend behaviour use the Solidity library feature if possible.
` morehref = 'http://solidity.readthedocs.io/en/develop/contracts.html#libraries' break case common.lowLevelCallTypes.DELEGATECALL: - text = `use of "delegatecall": the use of low level "delegatecall" should be avoided whenever possible. + text = yo`use of "delegatecall": the use of low level "delegatecall" should be avoided whenever possible. External code that is called can change the state of the calling contract and send ether form the caller's balance. - If this is wantend behaviour use the Solidity library feature if possible.
` + If this is wantend behaviour use the Solidity library feature if possible.
` morehref = 'http://solidity.readthedocs.io/en/develop/contracts.html#libraries' break case common.lowLevelCallTypes.SEND: - text = `use of "send": "send" does not throw an exception when not successful, make sure you deal with the failure case accordingly. + text = yo`use of "send": "send" does not throw an exception when not successful, make sure you deal with the failure case accordingly. Use "transfer" whenever failure of the ether transfer should rollback the whole transaction. Note: if you "send/transfer" ether to a contract the fallback function is called, the callees fallback function is very limited due to the limited amount of gas provided by "send/transfer". - No state changes are possible but the callee can log the event or revert the transfer. "send/transfer" is syntactic sugar for a "call" to the fallback function with 2300 gas and a specified ether value.
` + No state changes are possible but the callee can log the event or revert the transfer. "send/transfer" is syntactic sugar for a "call" to the fallback function with 2300 gas and a specified ether value.
` morehref = 'http://solidity.readthedocs.io/en/develop/security-considerations.html#sending-and-receiving-ether' break } diff --git a/src/app/staticanalysis/modules/similarVariableNames.js b/src/app/staticanalysis/modules/similarVariableNames.js index 458c368167..ea2c80dbf5 100644 --- a/src/app/staticanalysis/modules/similarVariableNames.js +++ b/src/app/staticanalysis/modules/similarVariableNames.js @@ -4,6 +4,7 @@ var categories = require('./categories') var common = require('./staticAnalysisCommon') var AbstractAst = require('./abstractAstView') var levenshtein = require('fast-levenshtein') +var yo = require('yo-yo') function similarVariableNames () { this.abstractAst = new AbstractAst() @@ -26,13 +27,20 @@ function report (contracts, multipleContractsWithSameName) { contracts.forEach((contract) => { contract.functions.forEach((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 hasModifiersComments = '' + if (hasModifiers) { + hasModifiersComments = yo`
Note: Modifiers are currently not considered by this static analysis.` + } + var multipleContractsWithSameNameComments = '' + if (multipleContractsWithSameName) { + multipleContractsWithSameNameComments = yo`
Note: Import aliases are currently not supported by this static analysis.` + } + var vars = getFunctionVariables(contract, func).map(common.getDeclaredVariableName) findSimilarVarNames(vars).map((sim) => { warnings.push({ - warning: `${funcName}: Variables have very similar names ${sim.var1} and ${sim.var2}. ${comments}`, + warning: yo`${funcName}: Variables have very similar names ${sim.var1} and ${sim.var2}. ${hasModifiersComments} ${multipleContractsWithSameNameComments}`, location: func.src }) }) diff --git a/src/app/staticanalysis/modules/thisLocal.js b/src/app/staticanalysis/modules/thisLocal.js index 2ca7a5975c..6a43c65035 100644 --- a/src/app/staticanalysis/modules/thisLocal.js +++ b/src/app/staticanalysis/modules/thisLocal.js @@ -2,6 +2,7 @@ var name = 'This on local calls: ' var desc = 'Invocation of local functions via this' var categories = require('./categories') var common = require('./staticAnalysisCommon') +var yo = require('yo-yo') function thisLocal () { this.warningNodes = [] @@ -14,7 +15,7 @@ thisLocal.prototype.visit = function (node) { thisLocal.prototype.report = function (compilationResults) { return this.warningNodes.map(function (item, i) { return { - warning: 'Use of "this" for local functions: Never use this to call functions in the same contract, it only consumes more gas than normal local calls.', + warning: yo`Use of "this" for local functions: Never use this to call functions in the same contract, it only consumes more gas than normal local calls.`, location: item.src, more: 'http://solidity.readthedocs.io/en/develop/control-structures.html#external-function-calls' } diff --git a/src/app/staticanalysis/modules/txOrigin.js b/src/app/staticanalysis/modules/txOrigin.js index 6209f77bac..df6346f449 100644 --- a/src/app/staticanalysis/modules/txOrigin.js +++ b/src/app/staticanalysis/modules/txOrigin.js @@ -1,6 +1,7 @@ var name = 'Transaction origin: ' var desc = 'Warn if tx.origin is used' var categories = require('./categories') +var yo = require('yo-yo') function txOrigin () { this.txOriginNodes = [] @@ -20,8 +21,8 @@ txOrigin.prototype.visit = function (node) { txOrigin.prototype.report = function () { return this.txOriginNodes.map(function (item, i) { return { - warning: `Use of tx.origin: "tx.origin" is useful only in very exceptional cases.
- If you use it for authentication, you usually want to replace it by "msg.sender", because otherwise any contract you call can act on your behalf.`, + warning: yo`Use of tx.origin: "tx.origin" is useful only in very exceptional cases.
+ If you use it for authentication, you usually want to replace it by "msg.sender", because otherwise any contract you call can act on your behalf.`, location: item.src } }) diff --git a/src/app/staticanalysis/staticAnalysisView.js b/src/app/staticanalysis/staticAnalysisView.js index 1b03e35bd3..fbbb78dba2 100644 --- a/src/app/staticanalysis/staticAnalysisView.js +++ b/src/app/staticanalysis/staticAnalysisView.js @@ -115,10 +115,11 @@ staticAnalysisView.prototype.run = function () { length: parseInt(split[1]) } location = self.appAPI.offsetToLineColumn(location, file) - location = self.lastCompilationResult.sourceList[file] + ':' + (location.start.line + 1) + ':' + (location.start.column + 1) + ':' + location = self.lastCompilationResult.sourceList[file] + ':' + (location.start.line + 1) + ':' + (location.start.column + 1) + ': ' } warningCount++ - self.appAPI.renderWarning(location + ' ' + item.warning + ((item.more) ? '
more' : ''), warningContainer, {type: 'warning', useSpan: true, isHTML: true}) + var msg = yo`${location} ${item.warning} ${item.more ? yo`
more` : yo``}` + self.appAPI.renderWarning(msg, warningContainer, {type: 'warning', useSpan: true}) }) }) if (warningContainer.html() === '') {