mirror
/
remix-project
mirror of https://github.com/ethereum/remix-project.git
2
0
Fork 0
Code Issues Releases Wiki Activity

Merge pull request #883 from soad003/bytesStringLength

Browse Source 
Static Analysis: warn when bytes(str).length is used to eval string l…
pull/5370/head
yann300 6 years ago committed by GitHub
parent 36f8a47612 75391bf5ce
commit e3b8f7e8c3
5 changed files with 138 additions and 21 deletions
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Show Stats Download Patch File Download Diff File
  1. 3
      remix-analyzer/src/solidity-analyzer/modules/list.js
  2. 19
      remix-analyzer/src/solidity-analyzer/modules/staticAnalysisCommon.js
  3. 34
      remix-analyzer/src/solidity-analyzer/modules/stringBytesLength.js
  4. 93
      remix-analyzer/test/analysis/staticAnalysisIntegration-test.js
  5. 10
      remix-analyzer/test/analysis/test-contracts/stringBytesLength.sol

3
remix-analyzer/src/solidity-analyzer/modules/list.js
Unescape View File

@ -14,5 +14,6 @@ module.exports = [
require('./guardConditions'),
require('./deleteDynamicArrays'),
require('./assignAndCompare'),
require('./erc20Decimals')
require('./erc20Decimals'),
require('./stringBytesLength')
]

19
remix-analyzer/src/solidity-analyzer/modules/staticAnalysisCommon.js
Unescape View File

@ -25,7 +25,8 @@ var nodeTypes = {
IFSTATEMENT: 'IfStatement',
FORSTATEMENT: 'ForStatement',
WHILESTATEMENT: 'WhileStatement',
DOWHILESTATEMENT: 'DoWhileStatement'
DOWHILESTATEMENT: 'DoWhileStatement',
ELEMENTARYTYPENAMEEXPRESSION: 'ElementaryTypeNameExpression'
}
var basicTypes = {
@ -871,6 +872,20 @@ function isTransfer (node) {
undefined, exactMatch(basicTypes.ADDRESS), exactMatch(lowLevelCallTypes.TRANSFER.ident))
}
function isStringToBytesConversion (node) {
return isExplicitCast(node, util.escapeRegExp('string *'), util.escapeRegExp('bytes'))
}
function isExplicitCast (node, castFromType, castToType) {
return nodeType(node, exactMatch(nodeTypes.FUNCTIONCALL)) && nrOfChildren(node, 2) &&
nodeType(node.children[0], exactMatch(nodeTypes.ELEMENTARYTYPENAMEEXPRESSION)) && name(node.children[0], castToType) &&
nodeType(node.children[1], exactMatch(nodeTypes.IDENTIFIER)) && expressionType(node.children[1], castFromType)
}
function isBytesLengthCheck (node) {
return isMemberAccess(node, exactMatch(util.escapeRegExp(basicTypes.UINT)), undefined, util.escapeRegExp('bytes *'), 'length')
}
// #################### Complex Node Identification - Private
function isMemberAccess (node, retType, accessor, accessorType, memberName) {
@ -1017,6 +1032,8 @@ module.exports = {
isAssertCall: isAssertCall,
isRequireCall: isRequireCall,
isIntDivision: isIntDivision,
isStringToBytesConversion: isStringToBytesConversion,
isBytesLengthCheck: isBytesLengthCheck,
// #################### Trivial Node Identification
isDeleteUnaryOperation: isDeleteUnaryOperation,

34
remix-analyzer/src/solidity-analyzer/modules/stringBytesLength.js
Unescape View File

@ -0,0 +1,34 @@
var name = 'String Length: '
var desc = 'Bytes length != String length'
var categories = require('./categories')
var common = require('./staticAnalysisCommon')
function stringBytesLength () {
this.stringToBytesConversions = []
this.bytesLengthChecks = []
}
stringBytesLength.prototype.visit = function (node) {
if (common.isStringToBytesConversion(node)) this.stringToBytesConversions.push(node)
else if (common.isBytesLengthCheck(node)) this.bytesLengthChecks.push(node)
}
stringBytesLength.prototype.report = function (compilationResults) {
if (this.stringToBytesConversions.length > 0 && this.bytesLengthChecks.length > 0) {
return [{
warning: 'Bytes and string length are not the same since strings are assumed to be UTF-8 encoded (according to the ABI defintion) therefore one character is not nessesarily encoded in one byte of data.',
location: this.bytesLengthChecks[0],
more: 'https://github.com/ethereum/wiki/wiki/Ethereum-Contract-ABI#argument-encoding'
}]
} else {
return []
}
}
module.exports = {
name: name,
description: desc,
category: categories.MISC,
Module: stringBytesLength
}

93
remix-analyzer/test/analysis/staticAnalysisIntegration-test.js
Unescape View File

@ -31,7 +31,8 @@ var testFiles = [
'deleteDynamicArray.sol',
'blockLevelCompare.sol',
'intDivisionTruncate.sol',
'ERC20.sol'
'ERC20.sol',
'stringBytesLength.sol'
]
var testFileAsts = {}
@ -68,7 +69,8 @@ test('Integration test thisLocal.js', function (t) {
'deleteDynamicArray.sol': 0,
'blockLevelCompare.sol': 0,
'intDivisionTruncate.sol': 0,
'ERC20.sol': 0
'ERC20.sol': 0,
'stringBytesLength.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
@ -103,7 +105,8 @@ test('Integration test checksEffectsInteraction.js', function (t) {
'deleteDynamicArray.sol': 0,
'blockLevelCompare.sol': 0,
'intDivisionTruncate.sol': 0,
'ERC20.sol': 0
'ERC20.sol': 0,
'stringBytesLength.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
@ -138,7 +141,8 @@ test('Integration test constantFunctions.js', function (t) {
'deleteDynamicArray.sol': 0,
'blockLevelCompare.sol': 0,
'intDivisionTruncate.sol': 0,
'ERC20.sol': 0
'ERC20.sol': 0,
'stringBytesLength.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
@ -173,7 +177,8 @@ test('Integration test inlineAssembly.js', function (t) {
'deleteDynamicArray.sol': 0,
'blockLevelCompare.sol': 0,
'intDivisionTruncate.sol': 0,
'ERC20.sol': 0
'ERC20.sol': 0,
'stringBytesLength.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
@ -208,7 +213,8 @@ test('Integration test txOrigin.js', function (t) {
'deleteDynamicArray.sol': 0,
'blockLevelCompare.sol': 0,
'intDivisionTruncate.sol': 0,
'ERC20.sol': 0
'ERC20.sol': 0,
'stringBytesLength.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
@ -243,7 +249,8 @@ test('Integration test gasCosts.js', function (t) {
'deleteDynamicArray.sol': 2,
'blockLevelCompare.sol': 1,
'intDivisionTruncate.sol': 1,
'ERC20.sol': 2
'ERC20.sol': 2,
'stringBytesLength.sol': 1
}
runModuleOnFiles(module, t, (file, report) => {
@ -278,7 +285,8 @@ test('Integration test similarVariableNames.js', function (t) {
'deleteDynamicArray.sol': 1,
'blockLevelCompare.sol': 0,
'intDivisionTruncate.sol': 0,
'ERC20.sol': 0
'ERC20.sol': 0,
'stringBytesLength.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
@ -313,7 +321,8 @@ test('Integration test inlineAssembly.js', function (t) {
'deleteDynamicArray.sol': 0,
'blockLevelCompare.sol': 0,
'intDivisionTruncate.sol': 0,
'ERC20.sol': 0
'ERC20.sol': 0,
'stringBytesLength.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
@ -348,7 +357,8 @@ test('Integration test blockTimestamp.js', function (t) {
'deleteDynamicArray.sol': 0,
'blockLevelCompare.sol': 0,
'intDivisionTruncate.sol': 0,
'ERC20.sol': 0
'ERC20.sol': 0,
'stringBytesLength.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
@ -383,7 +393,8 @@ test('Integration test lowLevelCalls.js', function (t) {
'deleteDynamicArray.sol': 0,
'blockLevelCompare.sol': 0,
'intDivisionTruncate.sol': 0,
'ERC20.sol': 0
'ERC20.sol': 0,
'stringBytesLength.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
@ -418,7 +429,8 @@ test('Integration test blockBlockhash.js', function (t) {
'deleteDynamicArray.sol': 0,
'blockLevelCompare.sol': 0,
'intDivisionTruncate.sol': 0,
'ERC20.sol': 0
'ERC20.sol': 0,
'stringBytesLength.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
@ -453,7 +465,8 @@ test('Integration test noReturn.js', function (t) {
'deleteDynamicArray.sol': 0,
'blockLevelCompare.sol': 0,
'intDivisionTruncate.sol': 0,
'ERC20.sol': 0
'ERC20.sol': 0,
'stringBytesLength.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
@ -487,8 +500,9 @@ test('Integration test selfdestruct.js', function (t) {
'selfdestruct.sol': 3,
'deleteDynamicArray.sol': 0,
'blockLevelCompare.sol': 0,
'ERC20.sol': 0,
'intDivisionTruncate.sol': 5,
'ERC20.sol': 0
'stringBytesLength.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
@ -523,7 +537,8 @@ test('Integration test guardConditions.js', function (t) {
'deleteDynamicArray.sol': 1,
'blockLevelCompare.sol': 0,
'intDivisionTruncate.sol': 1,
'ERC20.sol': 0
'ERC20.sol': 0,
'stringBytesLength.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
@ -558,7 +573,8 @@ test('Integration test deleteDynamicArrays.js', function (t) {
'deleteDynamicArray.sol': 2,
'blockLevelCompare.sol': 0,
'intDivisionTruncate.sol': 0,
'ERC20.sol': 0
'ERC20.sol': 0,
'stringBytesLength.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
@ -593,7 +609,8 @@ test('Integration test assignAndCompare.js', function (t) {
'deleteDynamicArray.sol': 0,
'blockLevelCompare.sol': 8,
'intDivisionTruncate.sol': 0,
'ERC20.sol': 0
'ERC20.sol': 0,
'stringBytesLength.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
@ -628,7 +645,8 @@ test('Integration test intDivisionTruncate.js', function (t) {
'deleteDynamicArray.sol': 0,
'blockLevelCompare.sol': 0,
'intDivisionTruncate.sol': 2,
'ERC20.sol': 0
'ERC20.sol': 0,
'stringBytesLength.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
@ -663,7 +681,8 @@ test('Integration test erc20Decimal.js', function (t) {
'deleteDynamicArray.sol': 0,
'blockLevelCompare.sol': 0,
'intDivisionTruncate.sol': 0,
'ERC20.sol': 1
'ERC20.sol': 1,
'stringBytesLength.sol': 0
}
runModuleOnFiles(module, t, (file, report) => {
@ -671,6 +690,42 @@ test('Integration test erc20Decimal.js', function (t) {
})
})
test('Integration test stringBytesLength.js', function (t) {
t.plan(testFiles.length)
var module = require('../../src/solidity-analyzer/modules/stringBytesLength')
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': 0,
'ERC20.sol': 0,
'stringBytesLength.sol': 1
}
runModuleOnFiles(module, t, (file, report) => {
t.equal(report.length, lengthCheck[file], `${file} has right amount of stringBytesLength warnings`)
})
})
// #################### Helpers
function runModuleOnFiles (module, t, cb) {
var statRunner = new StatRunner()

10
remix-analyzer/test/analysis/test-contracts/stringBytesLength.sol
Unescape View File

@ -0,0 +1,10 @@
pragma solidity ^0.4.17;
contract bytesString {
function length(string a) public pure returns(uint) {
bytes memory x = bytes(a);
return x.length;
}
}
Write Preview
Loading…
Cancel
Save