diff --git a/remix-solidity/src/analysis/modules/intDivisionTruncate.js b/remix-solidity/src/analysis/modules/intDivisionTruncate.js new file mode 100644 index 0000000000..51a3199a99 --- /dev/null +++ b/remix-solidity/src/analysis/modules/intDivisionTruncate.js @@ -0,0 +1,28 @@ +var name = 'Data Trucated: ' +var desc = 'Division on int/uint values truncates the result.' +var categories = require('./categories') +var common = require('./staticAnalysisCommon') + +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(function (item, i) { + return { + warning: 'Division of integer values yields an integer value again. That means eg. a / 100 = 0 instead of 0.a 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, + Module: intDivitionTruncate +} diff --git a/remix-solidity/src/analysis/modules/staticAnalysisCommon.js b/remix-solidity/src/analysis/modules/staticAnalysisCommon.js index dc0bd2787d..51929708d0 100644 --- a/remix-solidity/src/analysis/modules/staticAnalysisCommon.js +++ b/remix-solidity/src/analysis/modules/staticAnalysisCommon.js @@ -615,6 +615,15 @@ function isConstructor (node) { ) } +/** + * True if node is integer division that truncates (not only int literals since those yield a rational value) + * @node {ASTNode} some AstNode + * @return {bool} + */ +function isIntDivision (node) { + return isBinaryOperation(node) && operator(node, exactMatch(util.escapeRegExp('/'))) && expressionType(node, util.escapeRegExp('int')) +} + /** * True if is block / SubScope has top level binops (e.g. that are not assigned to anything, most of the time confused compare instead of assign) * @node {ASTNode} some AstNode @@ -982,6 +991,7 @@ module.exports = { isSelfdestructCall: isSelfdestructCall, isAssertCall: isAssertCall, isRequireCall: isRequireCall, + isIntDivision: isIntDivision, // #################### Trivial Node Identification isDeleteUnaryOperation: isDeleteUnaryOperation, diff --git a/remix-solidity/test/analysis/staticAnalysisIntegration-test.js b/remix-solidity/test/analysis/staticAnalysisIntegration-test.js index b03df5e05b..3b1f31308a 100644 --- a/remix-solidity/test/analysis/staticAnalysisIntegration-test.js +++ b/remix-solidity/test/analysis/staticAnalysisIntegration-test.js @@ -29,7 +29,8 @@ var testFiles = [ 'forgottenReturn.sol', 'selfdestruct.sol', 'deleteDynamicArray.sol', - 'blockLevelCompare.sol' + 'blockLevelCompare.sol', + 'intDivisionTruncate.sol' ] var testFileAsts = {} @@ -64,7 +65,8 @@ test('Integration test thisLocal.js', function (t) { 'forgottenReturn.sol': 0, 'selfdestruct.sol': 0, 'deleteDynamicArray.sol': 0, - 'blockLevelCompare.sol': 0 + 'blockLevelCompare.sol': 0, + 'intDivisionTruncate.sol': 0 } runModuleOnFiles(module, t, (file, report) => { @@ -97,7 +99,8 @@ test('Integration test checksEffectsInteraction.js', function (t) { 'forgottenReturn.sol': 0, 'selfdestruct.sol': 0, 'deleteDynamicArray.sol': 0, - 'blockLevelCompare.sol': 0 + 'blockLevelCompare.sol': 0, + 'intDivisionTruncate.sol': 0 } runModuleOnFiles(module, t, (file, report) => { @@ -130,7 +133,8 @@ test('Integration test constantFunctions.js', function (t) { 'forgottenReturn.sol': 0, 'selfdestruct.sol': 1, 'deleteDynamicArray.sol': 0, - 'blockLevelCompare.sol': 0 + 'blockLevelCompare.sol': 0, + 'intDivisionTruncate.sol': 0 } runModuleOnFiles(module, t, (file, report) => { @@ -163,7 +167,8 @@ test('Integration test inlineAssembly.js', function (t) { 'forgottenReturn.sol': 0, 'selfdestruct.sol': 0, 'deleteDynamicArray.sol': 0, - 'blockLevelCompare.sol': 0 + 'blockLevelCompare.sol': 0, + 'intDivisionTruncate.sol': 0 } runModuleOnFiles(module, t, (file, report) => { @@ -196,7 +201,8 @@ test('Integration test txOrigin.js', function (t) { 'forgottenReturn.sol': 0, 'selfdestruct.sol': 0, 'deleteDynamicArray.sol': 0, - 'blockLevelCompare.sol': 0 + 'blockLevelCompare.sol': 0, + 'intDivisionTruncate.sol': 0 } runModuleOnFiles(module, t, (file, report) => { @@ -229,7 +235,8 @@ test('Integration test gasCosts.js', function (t) { 'forgottenReturn.sol': 3, 'selfdestruct.sol': 0, 'deleteDynamicArray.sol': 2, - 'blockLevelCompare.sol': 1 + 'blockLevelCompare.sol': 1, + 'intDivisionTruncate.sol': 1 } runModuleOnFiles(module, t, (file, report) => { @@ -262,7 +269,8 @@ test('Integration test similarVariableNames.js', function (t) { 'forgottenReturn.sol': 0, 'selfdestruct.sol': 0, 'deleteDynamicArray.sol': 1, - 'blockLevelCompare.sol': 0 + 'blockLevelCompare.sol': 0, + 'intDivisionTruncate.sol': 0 } runModuleOnFiles(module, t, (file, report) => { @@ -295,7 +303,8 @@ test('Integration test inlineAssembly.js', function (t) { 'forgottenReturn.sol': 0, 'selfdestruct.sol': 0, 'deleteDynamicArray.sol': 0, - 'blockLevelCompare.sol': 0 + 'blockLevelCompare.sol': 0, + 'intDivisionTruncate.sol': 0 } runModuleOnFiles(module, t, (file, report) => { @@ -328,7 +337,8 @@ test('Integration test blockTimestamp.js', function (t) { 'forgottenReturn.sol': 0, 'selfdestruct.sol': 0, 'deleteDynamicArray.sol': 0, - 'blockLevelCompare.sol': 0 + 'blockLevelCompare.sol': 0, + 'intDivisionTruncate.sol': 0 } runModuleOnFiles(module, t, (file, report) => { @@ -361,7 +371,8 @@ test('Integration test lowLevelCalls.js', function (t) { 'forgottenReturn.sol': 0, 'selfdestruct.sol': 0, 'deleteDynamicArray.sol': 0, - 'blockLevelCompare.sol': 0 + 'blockLevelCompare.sol': 0, + 'intDivisionTruncate.sol': 0 } runModuleOnFiles(module, t, (file, report) => { @@ -394,7 +405,8 @@ test('Integration test blockBlockhash.js', function (t) { 'forgottenReturn.sol': 0, 'selfdestruct.sol': 0, 'deleteDynamicArray.sol': 0, - 'blockLevelCompare.sol': 0 + 'blockLevelCompare.sol': 0, + 'intDivisionTruncate.sol': 0 } runModuleOnFiles(module, t, (file, report) => { @@ -427,7 +439,8 @@ test('Integration test noReturn.js', function (t) { 'forgottenReturn.sol': 1, 'selfdestruct.sol': 0, 'deleteDynamicArray.sol': 0, - 'blockLevelCompare.sol': 0 + 'blockLevelCompare.sol': 0, + 'intDivisionTruncate.sol': 0 } runModuleOnFiles(module, t, (file, report) => { @@ -460,7 +473,8 @@ test('Integration test selfdestruct.js', function (t) { 'forgottenReturn.sol': 0, 'selfdestruct.sol': 2, 'deleteDynamicArray.sol': 0, - 'blockLevelCompare.sol': 0 + 'blockLevelCompare.sol': 0, + 'intDivisionTruncate.sol': 1 } runModuleOnFiles(module, t, (file, report) => { @@ -493,7 +507,8 @@ test('Integration test guardConditions.js', function (t) { 'forgottenReturn.sol': 0, 'selfdestruct.sol': 0, 'deleteDynamicArray.sol': 1, - 'blockLevelCompare.sol': 0 + 'blockLevelCompare.sol': 0, + 'intDivisionTruncate.sol': 1 } runModuleOnFiles(module, t, (file, report) => { @@ -526,7 +541,8 @@ test('Integration test deleteDynamicArrays.js', function (t) { 'forgottenReturn.sol': 0, 'selfdestruct.sol': 0, 'deleteDynamicArray.sol': 2, - 'blockLevelCompare.sol': 0 + 'blockLevelCompare.sol': 0, + 'intDivisionTruncate.sol': 0 } runModuleOnFiles(module, t, (file, report) => { @@ -559,7 +575,8 @@ test('Integration test assignAndCompare.js', function (t) { 'forgottenReturn.sol': 0, 'selfdestruct.sol': 0, 'deleteDynamicArray.sol': 0, - 'blockLevelCompare.sol': 8 + 'blockLevelCompare.sol': 8, + 'intDivisionTruncate.sol': 0 } runModuleOnFiles(module, t, (file, report) => { @@ -567,6 +584,40 @@ test('Integration test assignAndCompare.js', function (t) { }) }) +test('Integration test intDivisionTruncate.js', function (t) { + t.plan(testFiles.length) + + var module = require('../../src/analysis/modules/intDivisionTruncate') + + var lengthCheck = { + 'KingOfTheEtherThrone.sol': 0, + 'assembly.sol': 0, + 'ballot.sol': 0, + 'ballot_reentrant.sol': 0, + 'ballot_withoutWarnings.sol': 0, + 'cross_contract.sol': 0, + 'inheritance.sol': 0, + 'modifier1.sol': 0, + 'modifier2.sol': 0, + 'notReentrant.sol': 0, + 'structReentrant.sol': 0, + 'thisLocal.sol': 0, + 'globals.sol': 0, + 'library.sol': 0, + 'transfer.sol': 0, + 'ctor.sol': 0, + 'forgottenReturn.sol': 0, + 'selfdestruct.sol': 0, + 'deleteDynamicArray.sol': 0, + 'blockLevelCompare.sol': 0, + 'intDivisionTruncate.sol': 2 + } + + runModuleOnFiles(module, t, (file, report) => { + t.equal(report.length, lengthCheck[file], `${file} has right amount of intDivisionTruncate warnings`) + }) +}) + // #################### Helpers function runModuleOnFiles (module, t, cb) { var statRunner = new StatRunner() diff --git a/remix-solidity/test/analysis/test-contracts/intDivisionTruncate.sol b/remix-solidity/test/analysis/test-contracts/intDivisionTruncate.sol new file mode 100644 index 0000000000..a7c30fbfd1 --- /dev/null +++ b/remix-solidity/test/analysis/test-contracts/intDivisionTruncate.sol @@ -0,0 +1,28 @@ +pragma solidity ^0.4.19; + +contract CharityCampaign { + mapping (address => uint) contributions; + int128 feePercentage; + uint p2; + address processor; + address beneficiary; + + function CharityCampaign(address _beneficiary, int128 _feePercentage) public { + processor = msg.sender; + beneficiary = _beneficiary; + feePercentage = _feePercentage; + } + + function contribute() payable public returns (uint feeCollected) { + uint fee = msg.value * uint256(feePercentage / 100); + fee = msg.value * (p2 / 100); + contributions[msg.sender] = msg.value - fee; + processor.transfer(fee); + return fee; + } + + function endCampaign() public { + require(msg.sender == processor || msg.sender == beneficiary); + selfdestruct(beneficiary); + } +} \ No newline at end of file