diff --git a/remix-analyzer/src/solidity-analyzer/modules/assignAndCompare.ts b/remix-analyzer/src/solidity-analyzer/modules/assignAndCompare.ts index 1a3ba2855e..be12461e2b 100644 --- a/remix-analyzer/src/solidity-analyzer/modules/assignAndCompare.ts +++ b/remix-analyzer/src/solidity-analyzer/modules/assignAndCompare.ts @@ -1,17 +1,17 @@ -import { MISC } from './categories' -const common = require('./staticAnalysisCommon') +import { default as category } from './categories' +import { isSubScopeWithTopLevelUnAssignedBinOp, getUnAssignedTopLevelBinOps } from './staticAnalysisCommon' import { default as algorithm } from './algorithmCategories' export class assignAndCompare { - warningNodes: any = [] + warningNodes: any[] = [] name = 'Result not used: ' description = 'The result of an operation was not used.' - category = MISC + category = category.MISC algorithm = algorithm.EXACT Module = this visit (node) { - if (common.isSubScopeWithTopLevelUnAssignedBinOp(node)) common.getUnAssignedTopLevelBinOps(node).forEach((n) => this.warningNodes.push(n)) + if (isSubScopeWithTopLevelUnAssignedBinOp(node)) getUnAssignedTopLevelBinOps(node).forEach((n) => this.warningNodes.push(n)) } report (compilationResults) { diff --git a/remix-analyzer/src/solidity-analyzer/modules/blockBlockhash.js b/remix-analyzer/src/solidity-analyzer/modules/blockBlockhash.js deleted file mode 100644 index aef99f70d0..0000000000 --- a/remix-analyzer/src/solidity-analyzer/modules/blockBlockhash.js +++ /dev/null @@ -1,34 +0,0 @@ -const name = 'Block.blockhash usage: ' -const desc = 'Semantics maybe unclear' -const categories = require('./categories') -const common = require('./staticAnalysisCommon') -const algo = require('./algorithmCategories') - -function blockBlockhash () { - this.warningNodes = [] -} - -blockBlockhash.prototype.visit = function (node) { - if (common.isBlockBlockHashAccess(node)) this.warningNodes.push(node) -} - -blockBlockhash.prototype.report = function (compilationResults) { - return this.warningNodes.map((item, i) => { - return { - warning: `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.`, - location: item.src - } - }) -} - -module.exports = { - name: name, - description: desc, - category: categories.SECURITY, - algorithm: algo.EXACT, - Module: blockBlockhash -} - diff --git a/remix-analyzer/src/solidity-analyzer/modules/blockBlockhash.ts b/remix-analyzer/src/solidity-analyzer/modules/blockBlockhash.ts new file mode 100644 index 0000000000..3f5163d6d9 --- /dev/null +++ b/remix-analyzer/src/solidity-analyzer/modules/blockBlockhash.ts @@ -0,0 +1,29 @@ +import { default as category } from './categories' +import { isBlockBlockHashAccess } from './staticAnalysisCommon' +import { default as algorithm } from './algorithmCategories' + +export class blockBlockhash { + warningNodes: any[] = [] + name = 'Block.blockhash usage: ' + desc = 'Semantics maybe unclear' + categories = category.SECURITY + algorithm = algorithm.EXACT + Module = this + + visit (node) { + if (isBlockBlockHashAccess(node)) this.warningNodes.push(node) + } + + report (compilationResults) { + return this.warningNodes.map((item, i) => { + return { + warning: `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.`, + location: item.src + } + }) + } +} + diff --git a/remix-analyzer/src/solidity-analyzer/modules/blockTimestamp.js b/remix-analyzer/src/solidity-analyzer/modules/blockTimestamp.js deleted file mode 100644 index a2d58af5c5..0000000000 --- a/remix-analyzer/src/solidity-analyzer/modules/blockTimestamp.js +++ /dev/null @@ -1,42 +0,0 @@ -const name = 'Block timestamp: ' -const desc = 'Semantics maybe unclear' -const categories = require('./categories') -const common = require('./staticAnalysisCommon') -const algo = require('./algorithmCategories') - -function blockTimestamp () { - this.warningNowNodes = [] - this.warningblockTimestampNodes = [] -} - -blockTimestamp.prototype.visit = function (node) { - if (common.isNowAccess(node)) this.warningNowNodes.push(node) - else if (common.isBlockTimestampAccess(node)) this.warningblockTimestampNodes.push(node) -} - -blockTimestamp.prototype.report = function (compilationResults) { - return this.warningNowNodes.map((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.`, - location: item.src, - more: 'http://solidity.readthedocs.io/en/develop/frequently-asked-questions.html#are-timestamps-now-block-timestamp-reliable' - } - }).concat(this.warningblockTimestampNodes.map((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.`, - location: item.src, - more: 'http://solidity.readthedocs.io/en/develop/frequently-asked-questions.html#are-timestamps-now-block-timestamp-reliable' - } - })) -} - -module.exports = { - name: name, - description: desc, - category: categories.SECURITY, - algorithm: algo.EXACT, - Module: blockTimestamp -} - diff --git a/remix-analyzer/src/solidity-analyzer/modules/blockTimestamp.ts b/remix-analyzer/src/solidity-analyzer/modules/blockTimestamp.ts new file mode 100644 index 0000000000..9acf24a55d --- /dev/null +++ b/remix-analyzer/src/solidity-analyzer/modules/blockTimestamp.ts @@ -0,0 +1,36 @@ +import { default as category } from './categories' +import { isNowAccess, isBlockTimestampAccess } from './staticAnalysisCommon' +import { default as algorithm } from './algorithmCategories' + +export class blockTimestamp { + warningNowNodes: any[] = [] + warningblockTimestampNodes: any[] = [] + name = 'Block timestamp: ' + desc = 'Semantics maybe unclear' + categories = category.SECURITY + algorithm = algorithm.EXACT + Module = this + + visit (node) { + if (isNowAccess(node)) this.warningNowNodes.push(node) + else if (isBlockTimestampAccess(node)) this.warningblockTimestampNodes.push(node) + } + + report (compilationResults) { + return this.warningNowNodes.map((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.`, + location: item.src, + more: 'http://solidity.readthedocs.io/en/develop/frequently-asked-questions.html#are-timestamps-now-block-timestamp-reliable' + } + }).concat(this.warningblockTimestampNodes.map((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.`, + location: item.src, + more: 'http://solidity.readthedocs.io/en/develop/frequently-asked-questions.html#are-timestamps-now-block-timestamp-reliable' + } + })) + } +} diff --git a/remix-analyzer/src/solidity-analyzer/modules/categories.js b/remix-analyzer/src/solidity-analyzer/modules/categories.ts similarity index 91% rename from remix-analyzer/src/solidity-analyzer/modules/categories.js rename to remix-analyzer/src/solidity-analyzer/modules/categories.ts index 4b3c89a53f..478c355693 100644 --- a/remix-analyzer/src/solidity-analyzer/modules/categories.js +++ b/remix-analyzer/src/solidity-analyzer/modules/categories.ts @@ -1,4 +1,4 @@ -module.exports = { +export default { SECURITY: {displayName: 'Security', id: 'SEC'}, GAS: {displayName: 'Gas & Economy', id: 'GAS'}, MISC: {displayName: 'Miscellaneous', id: 'MISC'}, diff --git a/remix-analyzer/src/solidity-analyzer/modules/deleteDynamicArrays.js b/remix-analyzer/src/solidity-analyzer/modules/deleteDynamicArrays.js deleted file mode 100644 index e36864bb29..0000000000 --- a/remix-analyzer/src/solidity-analyzer/modules/deleteDynamicArrays.js +++ /dev/null @@ -1,31 +0,0 @@ -const name = 'Delete on dynamic Array: ' -const desc = 'Use require and appropriately' -const categories = require('./categories') -const common = require('./staticAnalysisCommon') -const algo = require('./algorithmCategories') - -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, - algorithm: algo.EXACT, - Module: deleteDynamicArrays -} diff --git a/remix-analyzer/src/solidity-analyzer/modules/deleteDynamicArrays.ts b/remix-analyzer/src/solidity-analyzer/modules/deleteDynamicArrays.ts new file mode 100644 index 0000000000..9ddbc32ede --- /dev/null +++ b/remix-analyzer/src/solidity-analyzer/modules/deleteDynamicArrays.ts @@ -0,0 +1,26 @@ +import { default as category } from './categories' +import { isDeleteOfDynamicArray } from './staticAnalysisCommon' +import { default as algorithm } from './algorithmCategories' + +export class deleteDynamicArrays { + rel: any = [] + name = 'Delete on dynamic Array: ' + desc = 'Use require and appropriately' + categories = category.GAS + algorithm = algorithm.EXACT + Module = this + + visit (node) { + if (isDeleteOfDynamicArray(node)) this.rel.push(node) + } + + report (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' + } + }) + } +} diff --git a/remix-analyzer/src/solidity-analyzer/modules/deleteFromDynamicArray.js b/remix-analyzer/src/solidity-analyzer/modules/deleteFromDynamicArray.js deleted file mode 100644 index 550c25c47a..0000000000 --- a/remix-analyzer/src/solidity-analyzer/modules/deleteFromDynamicArray.js +++ /dev/null @@ -1,29 +0,0 @@ -const name = 'Delete from dynamic Array: ' -const desc = 'Using delete on an array leaves a gap' -const categories = require('./categories') -const common = require('./staticAnalysisCommon') - -function deleteFromDynamicArray () { - this.relevantNodes = [] -} - -deleteFromDynamicArray.prototype.visit = function (node) { - if (common.isDeleteFromDynamicArray(node) && !common.isMappingIndexAccess(node.children[0])) 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 -} diff --git a/remix-analyzer/src/solidity-analyzer/modules/deleteFromDynamicArray.ts b/remix-analyzer/src/solidity-analyzer/modules/deleteFromDynamicArray.ts new file mode 100644 index 0000000000..12de067073 --- /dev/null +++ b/remix-analyzer/src/solidity-analyzer/modules/deleteFromDynamicArray.ts @@ -0,0 +1,24 @@ +import { default as category } from './categories' +import { isDeleteFromDynamicArray, isMappingIndexAccess } from './staticAnalysisCommon' + +export class deleteFromDynamicArray { + relevantNodes: any[] = [] + name = 'Delete from dynamic Array: ' + desc = 'Using delete on an array leaves a gap' + categories = category.MISC + Module = this + + visit (node) { + if (isDeleteFromDynamicArray(node) && !isMappingIndexAccess(node.children[0])) this.relevantNodes.push(node) + } + + report (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' + } + }) + } +} diff --git a/remix-analyzer/src/solidity-analyzer/modules/etherTransferInLoop.js b/remix-analyzer/src/solidity-analyzer/modules/etherTransferInLoop.js deleted file mode 100644 index 3c5298e341..0000000000 --- a/remix-analyzer/src/solidity-analyzer/modules/etherTransferInLoop.js +++ /dev/null @@ -1,41 +0,0 @@ -const name = 'Ether transfer in a loop: ' -const desc = 'Avoid transferring Ether to multiple addresses in a loop' -const categories = require('./categories') -const common = require('./staticAnalysisCommon') - -function etherTransferInLoop () { - this.relevantNodes = [] -} - -etherTransferInLoop.prototype.visit = function (node) { - if (common.isLoop(node)) { - let transferNodes = [] - const loopBlockStartIndex = common.getLoopBlockStartIndex(node) - if (common.isBlock(node.children[loopBlockStartIndex])) { - transferNodes = node.children[loopBlockStartIndex].children - .filter(child => (common.isExpressionStatement(child) && - child.children[0].name === 'FunctionCall' && - common.isTransfer(child.children[0].children[0]))) - if (transferNodes.length > 0) { - this.relevantNodes.push(...transferNodes) - } - } - } -} - -etherTransferInLoop.prototype.report = function (compilationResults) { - return this.relevantNodes.map((node) => { - return { - warning: 'Ether payout should not be done in a loop: 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. If required then make sure that number of iterations are low and you trust each address involved.', - location: node.src, - more: 'https://solidity.readthedocs.io/en/latest/security-considerations.html#gas-limit-and-loops' - } - }) -} - -module.exports = { - name: name, - description: desc, - category: categories.GAS, - Module: etherTransferInLoop -} diff --git a/remix-analyzer/src/solidity-analyzer/modules/etherTransferInLoop.ts b/remix-analyzer/src/solidity-analyzer/modules/etherTransferInLoop.ts new file mode 100644 index 0000000000..769a18393c --- /dev/null +++ b/remix-analyzer/src/solidity-analyzer/modules/etherTransferInLoop.ts @@ -0,0 +1,36 @@ +import { default as category } from './categories' +import { isLoop, isBlock, getLoopBlockStartIndex, isExpressionStatement, isTransfer } from './staticAnalysisCommon' + +export class etherTransferInLoop { + relevantNodes: any[] = [] + name = 'Ether transfer in a loop: ' + desc = 'Avoid transferring Ether to multiple addresses in a loop' + category = category.GAS + Module = this + + visit (node) { + if (isLoop(node)) { + let transferNodes = [] + const loopBlockStartIndex = getLoopBlockStartIndex(node) + if (isBlock(node.children[loopBlockStartIndex])) { + transferNodes = node.children[loopBlockStartIndex].children + .filter(child => (isExpressionStatement(child) && + child.children[0].name === 'FunctionCall' && + isTransfer(child.children[0].children[0]))) + if (transferNodes.length > 0) { + this.relevantNodes.push(...transferNodes) + } + } + } + } + + report (compilationResults) { + return this.relevantNodes.map((node) => { + return { + warning: 'Ether payout should not be done in a loop: 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. If required then make sure that number of iterations are low and you trust each address involved.', + location: node.src, + more: 'https://solidity.readthedocs.io/en/latest/security-considerations.html#gas-limit-and-loops' + } + }) + } +} diff --git a/remix-analyzer/src/solidity-analyzer/modules/forLoopIteratesOverDynamicArray.js b/remix-analyzer/src/solidity-analyzer/modules/forLoopIteratesOverDynamicArray.js deleted file mode 100644 index 64cd74b09a..0000000000 --- a/remix-analyzer/src/solidity-analyzer/modules/forLoopIteratesOverDynamicArray.js +++ /dev/null @@ -1,40 +0,0 @@ -const name = 'For loop iterates over dynamic array: ' -const desc = 'The number of \'for\' loop iterations depends on dynamic array\'s size' -const categories = require('./categories') -const { isForLoop, isDynamicArrayLengthAccess, isBinaryOperation } = require('./staticAnalysisCommon') - -function forLoopIteratesOverDynamicArray () { - this.relevantNodes = [] -} - -forLoopIteratesOverDynamicArray.prototype.visit = function (node) { - if (isForLoop(node)) { - // Access 'condition' node of 'for' loop statement - const forLoopConditionNode = node.children[1] - // Access right side of condition as its children - const conditionChildrenNode = forLoopConditionNode.children[1] - // Check if it is a binary operation. if yes, check if its children node access length of dynamic array - if (isBinaryOperation(conditionChildrenNode) && isDynamicArrayLengthAccess(conditionChildrenNode.children[0])) { - this.relevantNodes.push(node) - } else if (isDynamicArrayLengthAccess(conditionChildrenNode)) { // else check if condition node itself access length of dynamic array - 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 -} diff --git a/remix-analyzer/src/solidity-analyzer/modules/forLoopIteratesOverDynamicArray.ts b/remix-analyzer/src/solidity-analyzer/modules/forLoopIteratesOverDynamicArray.ts new file mode 100644 index 0000000000..bca8cc4bae --- /dev/null +++ b/remix-analyzer/src/solidity-analyzer/modules/forLoopIteratesOverDynamicArray.ts @@ -0,0 +1,35 @@ +import { default as category } from './categories' +const { isForLoop, isDynamicArrayLengthAccess, isBinaryOperation } = require('./staticAnalysisCommon') + +export class forLoopIteratesOverDynamicArray { + relevantNodes: any[] = [] + name = 'For loop iterates over dynamic array: ' + desc = 'The number of \'for\' loop iterations depends on dynamic array\'s size' + categories = category.GAS + Module = this + + visit (node) { + if (isForLoop(node)) { + // Access 'condition' node of 'for' loop statement + const forLoopConditionNode = node.children[1] + // Access right side of condition as its children + const conditionChildrenNode = forLoopConditionNode.children[1] + // Check if it is a binary operation. if yes, check if its children node access length of dynamic array + if (isBinaryOperation(conditionChildrenNode) && isDynamicArrayLengthAccess(conditionChildrenNode.children[0])) { + this.relevantNodes.push(node) + } else if (isDynamicArrayLengthAccess(conditionChildrenNode)) { // else check if condition node itself access length of dynamic array + this.relevantNodes.push(node) + } + } + } + + report (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' + } + }) + } +} diff --git a/remix-analyzer/src/solidity-analyzer/modules/guardConditions.js b/remix-analyzer/src/solidity-analyzer/modules/guardConditions.js deleted file mode 100644 index 21b2065f60..0000000000 --- a/remix-analyzer/src/solidity-analyzer/modules/guardConditions.js +++ /dev/null @@ -1,31 +0,0 @@ -const name = 'Guard Conditions: ' -const desc = 'Use require and appropriately' -const categories = require('./categories') -const common = require('./staticAnalysisCommon') -const algo = require('./algorithmCategories') - -function guardConditions () { - this.guards = [] -} - -guardConditions.prototype.visit = function (node) { - if (common.isRequireCall(node) || common.isAssertCall(node)) this.guards.push(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.', - more: 'http://solidity.readthedocs.io/en/develop/control-structures.html#error-handling-assert-require-revert-and-exceptions' - }] - } - return [] -} - -module.exports = { - name: name, - description: desc, - category: categories.MISC, - algorithm: algo.EXACT, - Module: guardConditions -} diff --git a/remix-analyzer/src/solidity-analyzer/modules/guardConditions.ts b/remix-analyzer/src/solidity-analyzer/modules/guardConditions.ts new file mode 100644 index 0000000000..919b08d3dd --- /dev/null +++ b/remix-analyzer/src/solidity-analyzer/modules/guardConditions.ts @@ -0,0 +1,26 @@ +import { default as category } from './categories' +import { isRequireCall, isAssertCall } from './staticAnalysisCommon' +import { default as algorithm } from './algorithmCategories' + +export class guardConditions { + guards: any[] = [] + name = 'Guard Conditions: ' + desc = 'Use require and appropriately' + categories = category.MISC + algorithm = algorithm.EXACT + Module = this + + visit (node) { + if (isRequireCall(node) || isAssertCall(node)) this.guards.push(node) + } + + report (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.', + more: 'http://solidity.readthedocs.io/en/develop/control-structures.html#error-handling-assert-require-revert-and-exceptions' + }] + } + return [] + } +} diff --git a/remix-analyzer/src/solidity-analyzer/modules/inlineAssembly.js b/remix-analyzer/src/solidity-analyzer/modules/inlineAssembly.js deleted file mode 100644 index 595d95cab5..0000000000 --- a/remix-analyzer/src/solidity-analyzer/modules/inlineAssembly.js +++ /dev/null @@ -1,32 +0,0 @@ -const name = 'Inline assembly: ' -const desc = 'Use of Inline Assembly' -const categories = require('./categories') -const common = require('./staticAnalysisCommon') -const algo = require('./algorithmCategories') - -function inlineAssembly () { - this.inlineAssNodes = [] -} - -inlineAssembly.prototype.visit = function (node) { - if (common.isInlineAssembly(node)) this.inlineAssNodes.push(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.`, - location: node.src, - more: 'http://solidity.readthedocs.io/en/develop/assembly.html#solidity-assembly' - } - }) -} - -module.exports = { - name: name, - description: desc, - category: categories.SECURITY, - algorithm: algo.EXACT, - Module: inlineAssembly -} diff --git a/remix-analyzer/src/solidity-analyzer/modules/inlineAssembly.ts b/remix-analyzer/src/solidity-analyzer/modules/inlineAssembly.ts new file mode 100644 index 0000000000..3b49193015 --- /dev/null +++ b/remix-analyzer/src/solidity-analyzer/modules/inlineAssembly.ts @@ -0,0 +1,27 @@ +import { default as category } from './categories' +import { isInlineAssembly } from './staticAnalysisCommon' +import { default as algorithm } from './algorithmCategories' + +export class inlineAssembly { + inlineAssNodes: any[] = [] + name = 'Inline assembly: ' + desc = 'Use of Inline Assembly' + categories = category.SECURITY + algorithm = algorithm.EXACT + Module = this + + visit (node) { + if (isInlineAssembly(node)) this.inlineAssNodes.push(node) + } + + report (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.`, + location: node.src, + more: 'http://solidity.readthedocs.io/en/develop/assembly.html#solidity-assembly' + } + }) + } +} diff --git a/remix-analyzer/src/solidity-analyzer/modules/intDivisionTruncate.js b/remix-analyzer/src/solidity-analyzer/modules/intDivisionTruncate.js deleted file mode 100644 index a5b9ec447a..0000000000 --- a/remix-analyzer/src/solidity-analyzer/modules/intDivisionTruncate.js +++ /dev/null @@ -1,30 +0,0 @@ -const name = 'Data Trucated: ' -const desc = 'Division on int/uint values truncates the result.' -const categories = require('./categories') -const common = require('./staticAnalysisCommon') -const algo = require('./algorithmCategories') - -function intDivitionTruncate () { - this.warningNodes = [] -} - -intDivitionTruncate.prototype.visit = function (node) { - if (common.isIntDivision(node)) this.warningNodes.push(node) -} - -intDivitionTruncate.prototype.report = function (compilationResults) { - return this.warningNodes.map((item, i) => { - return { - warning: 'Division of integer values yields an integer value again. That means e.g. 10 / 100 = 0 instead of 0.1 since the result is an integer again. This does not hold for division of (only) literal values since those yield rational constants.', - location: item.src - } - }) -} - -module.exports = { - name: name, - description: desc, - category: categories.MISC, - algorithm: algo.EXACT, - Module: intDivitionTruncate -} diff --git a/remix-analyzer/src/solidity-analyzer/modules/intDivisionTruncate.ts b/remix-analyzer/src/solidity-analyzer/modules/intDivisionTruncate.ts new file mode 100644 index 0000000000..aff9276ced --- /dev/null +++ b/remix-analyzer/src/solidity-analyzer/modules/intDivisionTruncate.ts @@ -0,0 +1,25 @@ +import { default as category } from './categories' +import { isIntDivision } from './staticAnalysisCommon' +import { default as algorithm } from './algorithmCategories' + +export class intDivitionTruncate { + warningNodes: any[] = [] + name = 'Data Trucated: ' + desc = 'Division on int/uint values truncates the result.' + categories = category.MISC + algorithm = algorithm.EXACT + Module = this + + visit (node) { + if (isIntDivision(node)) this.warningNodes.push(node) + } + + report (compilationResults) { + return this.warningNodes.map((item, i) => { + return { + warning: 'Division of integer values yields an integer value again. That means e.g. 10 / 100 = 0 instead of 0.1 since the result is an integer again. This does not hold for division of (only) literal values since those yield rational constants.', + location: item.src + } + }) + } +} diff --git a/remix-analyzer/src/solidity-analyzer/modules/lowLevelCalls.js b/remix-analyzer/src/solidity-analyzer/modules/lowLevelCalls.js deleted file mode 100644 index 7ee2b5d971..0000000000 --- a/remix-analyzer/src/solidity-analyzer/modules/lowLevelCalls.js +++ /dev/null @@ -1,72 +0,0 @@ -const name = 'Low level calls: ' -const desc = 'Semantics maybe unclear' -const categories = require('./categories') -const common = require('./staticAnalysisCommon') -const algo = require('./algorithmCategories') - -function lowLevelCalls () { - this.llcNodes = [] -} - -lowLevelCalls.prototype.visit = function (node) { - if (common.isLowLevelCallInst(node)) { - this.llcNodes.push({node: node, type: common.lowLevelCallTypes.CALL}) - } else if (common.isLowLevelCallInst050(node)) { - this.llcNodes.push({node: node, type: common.lowLevelCallTypes.CALL}) - } else if (common.isLowLevelCallcodeInst(node)) { - this.llcNodes.push({node: node, type: common.lowLevelCallTypes.CALLCODE}) - } else if (common.isLowLevelDelegatecallInst(node)) { - this.llcNodes.push({node: node, type: common.lowLevelCallTypes.DELEGATECALL}) - } else if (common.isLowLevelSendInst(node)) { - this.llcNodes.push({node: node, type: common.lowLevelCallTypes.SEND}) - } else if (common.isLowLevelSendInst050(node)) { - this.llcNodes.push({node: node, type: common.lowLevelCallTypes.SEND}) - } else if (common.isLLDelegatecallInst050(node)) { - this.llcNodes.push({node: node, type: common.lowLevelCallTypes.DELEGATECALL}) - } -} - -lowLevelCalls.prototype.report = function (compilationResults) { - return this.llcNodes.map((item, i) => { - let text = '' - let morehref = null - switch (item.type) { - case common.lowLevelCallTypes.CALL: - text = `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.` - 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. - External code that is called can change the state of the calling contract and send ether from the caller's balance. - If this is wanted 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. - External code that is called can change the state of the calling contract and send ether from the caller's balance. - If this is wanted 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. - 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.` - morehref = 'http://solidity.readthedocs.io/en/develop/security-considerations.html#sending-and-receiving-ether' - break - } - return { warning: text, more: morehref, location: item.node.src } - }) -} - -module.exports = { - name: name, - description: desc, - category: categories.SECURITY, - algorithm: algo.EXACT, - Module: lowLevelCalls -} - diff --git a/remix-analyzer/src/solidity-analyzer/modules/lowLevelCalls.ts b/remix-analyzer/src/solidity-analyzer/modules/lowLevelCalls.ts new file mode 100644 index 0000000000..889c751ae5 --- /dev/null +++ b/remix-analyzer/src/solidity-analyzer/modules/lowLevelCalls.ts @@ -0,0 +1,68 @@ +import { default as category } from './categories' +import { isLowLevelCallInst, isLowLevelCallInst050, isLowLevelCallcodeInst, isLowLevelDelegatecallInst, + isLowLevelSendInst, isLowLevelSendInst050, isLLDelegatecallInst050, lowLevelCallTypes } from './staticAnalysisCommon' +import { default as algorithm } from './algorithmCategories' + +export class lowLevelCalls { + llcNodes: any[] = [] + name = 'Low level calls: ' + desc = 'Semantics maybe unclear' + categories = category.SECURITY + algorithm = algorithm.EXACT + Module = this + + visit (node) { + if (isLowLevelCallInst(node)) { + this.llcNodes.push({node: node, type: lowLevelCallTypes.CALL}) + } else if (isLowLevelCallInst050(node)) { + this.llcNodes.push({node: node, type: lowLevelCallTypes.CALL}) + } else if (isLowLevelCallcodeInst(node)) { + this.llcNodes.push({node: node, type: lowLevelCallTypes.CALLCODE}) + } else if (isLowLevelDelegatecallInst(node)) { + this.llcNodes.push({node: node, type: lowLevelCallTypes.DELEGATECALL}) + } else if (isLowLevelSendInst(node)) { + this.llcNodes.push({node: node, type: lowLevelCallTypes.SEND}) + } else if (isLowLevelSendInst050(node)) { + this.llcNodes.push({node: node, type: lowLevelCallTypes.SEND}) + } else if (isLLDelegatecallInst050(node)) { + this.llcNodes.push({node: node, type: lowLevelCallTypes.DELEGATECALL}) + } + } + + report (compilationResults) { + return this.llcNodes.map((item, i) => { + let text = '' + let morehref: any = null + switch (item.type) { + case lowLevelCallTypes.CALL: + text = `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.` + 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 lowLevelCallTypes.CALLCODE: + text = `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 from the caller's balance. + If this is wanted behaviour use the Solidity library feature if possible.` + morehref = 'http://solidity.readthedocs.io/en/develop/contracts.html#libraries' + break + case lowLevelCallTypes.DELEGATECALL: + text = `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 from the caller's balance. + If this is wanted behaviour use the Solidity library feature if possible.` + morehref = 'http://solidity.readthedocs.io/en/develop/contracts.html#libraries' + break + case lowLevelCallTypes.SEND: + text = `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.` + morehref = 'http://solidity.readthedocs.io/en/develop/security-considerations.html#sending-and-receiving-ether' + break + } + return { warning: text, more: morehref, location: item.node.src } + }) + } +} +