Commit 4076d582 authored by chriseth's avatar chriseth Committed by GitHub

Merge pull request #560 from soad003/master

Static Analysis: modules inlineAssembly, blockhash, blocktimestamp, s…
parents 94bc7811 a6d36585
...@@ -45,7 +45,8 @@ ...@@ -45,7 +45,8 @@
"watchify": "^3.9.0", "watchify": "^3.9.0",
"web3": "^0.18.0", "web3": "^0.18.0",
"webworkify": "^1.2.1", "webworkify": "^1.2.1",
"yo-yo": "^1.2.2" "yo-yo": "^1.2.2",
"fast-levenshtein": "^2.0.6"
}, },
"repository": { "repository": {
"type": "git", "type": "git",
......
var name = 'block.blockhash usage'
var desc = 'semantics maybe unclear'
var categories = require('./categories')
var common = require('./staticAnalysisCommon')
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(function (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,
Module: blockBlockhash
}
var name = 'block timestamp'
var desc = 'semantics maybe unclear'
var categories = require('./categories')
var common = require('./staticAnalysisCommon')
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(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.`,
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.`,
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,
Module: blockTimestamp
}
...@@ -3,10 +3,10 @@ module.exports = [ ...@@ -3,10 +3,10 @@ module.exports = [
require('./gasCosts'), require('./gasCosts'),
require('./thisLocal'), require('./thisLocal'),
require('./checksEffectsInteraction'), require('./checksEffectsInteraction'),
require('./constantFunctions') require('./constantFunctions'),
// require('./similarVariableNames.js'), require('./similarVariableNames.js'),
// require('./inlineAssembly'), require('./inlineAssembly'),
// require('./blockTimestamp'), require('./blockTimestamp'),
// require('./lowLevelCalls'), require('./lowLevelCalls'),
// require('./blockBlockhash') require('./blockBlockhash')
] ]
var name = 'low level calls'
var desc = 'semantics maybe unclear'
var categories = require('./categories')
var common = require('./staticAnalysisCommon')
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.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})
}
}
lowLevelCalls.prototype.report = function (compilationResults) {
return this.llcNodes.map(function (item, i) {
var text = ''
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. It can lead to unexpected behavior if return value is not handled properly. Please use Direct Calls via specifying the called contract's interface.<br />`
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 form the caller's balance. If this is wantend behaviour use the Solidity library feature if possible.<br />`
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 form the caller's balance. If this is wantend behaviour use the Solidity library feature if possible.<br />`
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. <br />`
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,
Module: lowLevelCalls
}
var name = 'Similar Variable Names'
var desc = 'Check if variable names are too similar'
var categories = require('./categories')
var common = require('./staticAnalysisCommon')
var AbstractAst = require('./abstractAstView')
var levenshtein = require('fast-levenshtein')
function similarVariableNames () {
this.abstractAst = new AbstractAst()
this.visit = this.abstractAst.build_visit(
(node) => false
)
this.report = this.abstractAst.build_report(report)
}
similarVariableNames.prototype.visit = function () { throw new Error('similarVariableNames.js no visit function set upon construction') }
similarVariableNames.prototype.report = function () { throw new Error('similarVariableNames.js no report function set upon construction') }
function report (contracts, multipleContractsWithSameName) {
var warnings = []
var hasModifiers = contracts.some((item) => item.modifiers.length > 0)
contracts.forEach((contract) => {
contract.functions.forEach((func) => {
var funcName = common.getFullQuallyfiedFuncDefinitionIdent(contract.node, func.node, func.parameters)
var comments = (hasModifiers) ? '<br/><i>Note:</i> Modifiers are currently not considered by this static analysis.' : ''
comments += (multipleContractsWithSameName) ? '<br/><i>Note:</i> 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: `<i>${funcName}</i>: Variables have very similar names <i>${sim.var1}</i> and <i>${sim.var2}<i>. ${comments}`,
location: func.src
})
})
})
})
return warnings
}
function findSimilarVarNames (vars) {
var similar = []
var comb = {}
vars.map((varName1) => vars.map((varName2) => {
if (varName1.length > 1 && varName2.length > 1 && varName2 !== varName1 && !isCommonPrefixedVersion(varName1, varName2) && !(comb[varName1 + ';' + varName2] || comb[varName2 + ';' + varName1])) {
comb[varName1 + ';' + varName2] = true
var distance = levenshtein.get(varName1, varName2)
if (distance <= 2) similar.push({ var1: varName1, var2: varName2, distance: distance })
}
}))
return similar
}
function isCommonPrefixedVersion (varName1, varName2) {
return (varName1.startsWith('_') && varName1.slice(1) === varName2) || (varName2.startsWith('_') && varName2.slice(1) === varName1)
}
function getFunctionVariables (contract, func) {
return contract.stateVariables.concat(func.localVariables)
}
module.exports = {
name: name,
description: desc,
category: categories.MISC,
Module: similarVariableNames
}
...@@ -403,7 +403,7 @@ function isStorageVariableDeclaration (node) { ...@@ -403,7 +403,7 @@ function isStorageVariableDeclaration (node) {
* @return {bool} * @return {bool}
*/ */
function isInteraction (node) { function isInteraction (node) {
return isLLCall(node) || isLLSend(node) || isExternalDirectCall(node) return isLLCall(node) || isLLSend(node) || isExternalDirectCall(node) || isTransfer(node)
} }
/** /**
......
...@@ -3,8 +3,6 @@ var test = require('tape') ...@@ -3,8 +3,6 @@ var test = require('tape')
var common = require('../../src/app/staticanalysis/modules/staticAnalysisCommon') var common = require('../../src/app/staticanalysis/modules/staticAnalysisCommon')
var utils = require('../../src/app/utils') var utils = require('../../src/app/utils')
// #################### helpers Test
test('staticAnalysisCommon.helpers.buildFunctionSignature', function (t) { test('staticAnalysisCommon.helpers.buildFunctionSignature', function (t) {
t.plan(7) t.plan(7)
...@@ -1255,7 +1253,6 @@ test('staticAnalysisCommon.isBuiltinFunctionCall', function (t) { ...@@ -1255,7 +1253,6 @@ test('staticAnalysisCommon.isBuiltinFunctionCall', function (t) {
t.notOk(common.isBuiltinFunctionCall(localCall), 'local call is not builtin') t.notOk(common.isBuiltinFunctionCall(localCall), 'local call is not builtin')
}) })
test('staticAnalysisCommon.isStorageVariableDeclaration', function (t) { test('staticAnalysisCommon.isStorageVariableDeclaration', function (t) {
t.plan(3) t.plan(3)
var node1 = { var node1 = {
......
...@@ -23,7 +23,9 @@ var testFiles = [ ...@@ -23,7 +23,9 @@ var testFiles = [
'structReentrant.sol', 'structReentrant.sol',
'thisLocal.sol', 'thisLocal.sol',
'globals.sol', 'globals.sol',
'library.sol' 'library.sol',
'transfer.sol',
'ctor.sol'
] ]
var testFileAsts = {} var testFileAsts = {}
...@@ -52,7 +54,9 @@ test('Integration test thisLocal.js', function (t) { ...@@ -52,7 +54,9 @@ test('Integration test thisLocal.js', function (t) {
'structReentrant.sol': 0, 'structReentrant.sol': 0,
'thisLocal.sol': 1, 'thisLocal.sol': 1,
'globals.sol': 0, 'globals.sol': 0,
'library.sol': 0 'library.sol': 0,
'transfer.sol': 0,
'ctor.sol': 0
} }
runModuleOnFiles(module, t, (file, report) => { runModuleOnFiles(module, t, (file, report) => {
...@@ -79,7 +83,9 @@ test('Integration test checksEffectsInteraction.js', function (t) { ...@@ -79,7 +83,9 @@ test('Integration test checksEffectsInteraction.js', function (t) {
'structReentrant.sol': 1, 'structReentrant.sol': 1,
'thisLocal.sol': 0, 'thisLocal.sol': 0,
'globals.sol': 1, 'globals.sol': 1,
'library.sol': 1 'library.sol': 1,
'transfer.sol': 1,
'ctor.sol': 0
} }
runModuleOnFiles(module, t, (file, report) => { runModuleOnFiles(module, t, (file, report) => {
...@@ -106,7 +112,9 @@ test('Integration test constantFunctions.js', function (t) { ...@@ -106,7 +112,9 @@ test('Integration test constantFunctions.js', function (t) {
'structReentrant.sol': 0, 'structReentrant.sol': 0,
'thisLocal.sol': 1, 'thisLocal.sol': 1,
'globals.sol': 0, 'globals.sol': 0,
'library.sol': 1 'library.sol': 1,
'transfer.sol': 0,
'ctor.sol': 0
} }
runModuleOnFiles(module, t, (file, report) => { runModuleOnFiles(module, t, (file, report) => {
...@@ -133,7 +141,9 @@ test('Integration test inlineAssembly.js', function (t) { ...@@ -133,7 +141,9 @@ test('Integration test inlineAssembly.js', function (t) {
'structReentrant.sol': 0, 'structReentrant.sol': 0,
'thisLocal.sol': 0, 'thisLocal.sol': 0,
'globals.sol': 0, 'globals.sol': 0,
'library.sol': 0 'library.sol': 0,
'transfer.sol': 0,
'ctor.sol': 0
} }
runModuleOnFiles(module, t, (file, report) => { runModuleOnFiles(module, t, (file, report) => {
...@@ -160,7 +170,9 @@ test('Integration test txOrigin.js', function (t) { ...@@ -160,7 +170,9 @@ test('Integration test txOrigin.js', function (t) {
'structReentrant.sol': 0, 'structReentrant.sol': 0,
'thisLocal.sol': 0, 'thisLocal.sol': 0,
'globals.sol': 1, 'globals.sol': 1,
'library.sol': 0 'library.sol': 0,
'transfer.sol': 0,
'ctor.sol': 0
} }
runModuleOnFiles(module, t, (file, report) => { runModuleOnFiles(module, t, (file, report) => {
...@@ -187,7 +199,9 @@ test('Integration test gasCosts.js', function (t) { ...@@ -187,7 +199,9 @@ test('Integration test gasCosts.js', function (t) {
'structReentrant.sol': 1, 'structReentrant.sol': 1,
'thisLocal.sol': 2, 'thisLocal.sol': 2,
'globals.sol': 1, 'globals.sol': 1,
'library.sol': 1 'library.sol': 1,
'transfer.sol': 1,
'ctor.sol': 0
} }
runModuleOnFiles(module, t, (file, report) => { runModuleOnFiles(module, t, (file, report) => {
...@@ -195,6 +209,151 @@ test('Integration test gasCosts.js', function (t) { ...@@ -195,6 +209,151 @@ test('Integration test gasCosts.js', function (t) {
}) })
}) })
test('Integration test similarVariableNames.js', function (t) {
t.plan(testFiles.length)
var module = require('../../src/app/staticanalysis/modules/similarVariableNames')
var lengthCheck = {
'KingOfTheEtherThrone.sol': 0,
'assembly.sol': 0,
'ballot.sol': 2,
'ballot_reentrant.sol': 3,
'ballot_withoutWarnings.sol': 0,
'cross_contract.sol': 0,
'inheritance.sol': 0,
'modifier1.sol': 0,
'modifier2.sol': 0,
'notReentrant.sol': 1,
'structReentrant.sol': 0,
'thisLocal.sol': 0,
'globals.sol': 0,
'library.sol': 0,
'transfer.sol': 0,
'ctor.sol': 1
}
runModuleOnFiles(module, t, (file, report) => {
t.equal(report.length, lengthCheck[file], `${file} has right amount of similarVariableNames warnings`)
})
})
test('Integration test inlineAssembly.js', function (t) {
t.plan(testFiles.length)
var module = require('../../src/app/staticanalysis/modules/inlineAssembly')
var lengthCheck = {
'KingOfTheEtherThrone.sol': 0,
'assembly.sol': 2,
'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
}
runModuleOnFiles(module, t, (file, report) => {
t.equal(report.length, lengthCheck[file], `${file} has right amount of inlineAssembly warnings`)
})
})
test('Integration test blockTimestamp.js', function (t) {
t.plan(testFiles.length)
var module = require('../../src/app/staticanalysis/modules/blockTimestamp')
var lengthCheck = {
'KingOfTheEtherThrone.sol': 1,
'assembly.sol': 0,
'ballot.sol': 0,
'ballot_reentrant.sol': 3,
'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': 2,
'library.sol': 0,
'transfer.sol': 0,
'ctor.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
t.equal(report.length, lengthCheck[file], `${file} has right amount of blockTimestamp warnings`)
})
})
test('Integration test lowLevelCalls.js', function (t) {
t.plan(testFiles.length)
var module = require('../../src/app/staticanalysis/modules/lowLevelCalls')
var lengthCheck = {
'KingOfTheEtherThrone.sol': 1,
'assembly.sol': 1,
'ballot.sol': 0,
'ballot_reentrant.sol': 7,
'ballot_withoutWarnings.sol': 0,
'cross_contract.sol': 1,
'inheritance.sol': 1,
'modifier1.sol': 0,
'modifier2.sol': 0,
'notReentrant.sol': 1,
'structReentrant.sol': 1,
'thisLocal.sol': 2,
'globals.sol': 1,
'library.sol': 1,
'transfer.sol': 0,
'ctor.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
t.equal(report.length, lengthCheck[file], `${file} has right amount of lowLevelCalls warnings`)
})
})
test('Integration test blockBlockhash.js', function (t) {
t.plan(testFiles.length)
var module = require('../../src/app/staticanalysis/modules/blockBlockhash')
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': 1,
'library.sol': 0,
'transfer.sol': 0,
'ctor.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
t.equal(report.length, lengthCheck[file], `${file} has right amount of blockBlockhash warnings`)
})
})
// #################### Helpers // #################### Helpers
function runModuleOnFiles (module, t, cb) { function runModuleOnFiles (module, t, cb) {
var statRunner = new StatRunner() var statRunner = new StatRunner()
......
contract c {
uint x;
uint x_abc;
function c(uint _x, uint _abc) {
x=_x;
x_abc=_abc;
}
}
\ No newline at end of file
contract c {
uint x;
function f(address r) {
r.transfer(1);
x = 2;
}
}
\ No newline at end of file
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment