Merge pull request #458 from crytic/dev-fix-function-signature

Revert incorrect parsing introduced with 0.6.11
pull/463/head
Feist Josselin 5 years ago committed by GitHub
commit f0bf1089d3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 2
      slither/core/cfg/node.py
  2. 22
      slither/core/declarations/function.py
  3. 2
      slither/printers/guidance/echidna.py
  4. 2
      slither/printers/summary/function_ids.py
  5. 20
      tests/expected_json/incorrect_equality.incorrect-equality.json
  6. 4
      tests/expected_json/incorrect_equality.incorrect-equality.txt
  7. 12
      tests/expected_json/reentrancy-0.5.1-events.reentrancy-events.json
  8. 2
      tests/expected_json/reentrancy-0.5.1-events.reentrancy-events.txt
  9. 20
      tests/expected_json/unused_return.unused-return.json
  10. 4
      tests/expected_json/unused_return.unused-return.txt
  11. 4
      tests/possible_paths/paths.txt

@ -813,7 +813,7 @@ class Node(SourceMapping, ChildFunction):
if isinstance(ir, LowLevelCall):
assert isinstance(ir.destination, (Variable, SolidityVariable))
self._low_level_calls.append((ir.destination, ir.function_name.value))
elif isinstance(ir, (HighLevelCall)) and not isinstance(ir, LibraryCall):
elif isinstance(ir, HighLevelCall) and not isinstance(ir, LibraryCall):
if isinstance(ir.destination.type, Contract):
self._high_level_calls.append((ir.destination.type, ir.function))
elif ir.destination == SolidityVariable('this'):

@ -3,8 +3,8 @@
"""
import logging
from collections import namedtuple
from itertools import groupby
from enum import Enum
from itertools import groupby
from slither.core.children.child_contract import ChildContract
from slither.core.children.child_inheritance import ChildInheritance
@ -16,7 +16,6 @@ from slither.core.expressions import (Identifier, IndexAccess, MemberAccess,
from slither.core.solidity_types import UserDefinedType
from slither.core.solidity_types.type import Type
from slither.core.source_mapping.source_mapping import SourceMapping
from slither.core.variables.state_variable import StateVariable
from slither.utils.utils import unroll
@ -728,12 +727,23 @@ class Function(ChildContract, ChildInheritance, SourceMapping):
###################################################################################
@staticmethod
def _convert_type_for_signature(t: Type):
from slither.core.declarations.contract import Contract
def _convert_type_for_solidity_signature(t: Type):
from slither.core.declarations import Contract
if isinstance(t, UserDefinedType) and isinstance(t.type, Contract):
return "address"
return str(t)
@property
def solidity_signature(self) -> str:
"""
Return a signature following the Solidity Standard
Contract and converted into address
:return: the solidity signature
"""
parameters = [self._convert_type_for_solidity_signature(x.type) for x in self.parameters]
return self.name + '(' + ','.join(parameters) + ')'
@property
def signature(self):
"""
@ -741,8 +751,8 @@ class Function(ChildContract, ChildInheritance, SourceMapping):
(name, list parameters type, list return values type)
"""
return (self.name,
[self._convert_type_for_signature(x.type) for x in self.parameters],
[self._convert_type_for_signature(x.type) for x in self.returns])
[str(x.type) for x in self.parameters],
[str(x.type) for x in self.returns])
@property
def signature_str(self):

@ -23,7 +23,7 @@ from slither.slithir.variables import Constant
def _get_name(f: Function) -> str:
if f.is_fallback or f.is_receive:
return f'()'
return f.full_name
return f.solidity_signature
def _extract_payable(slither: Slither) -> Dict[str, List[str]]:

@ -27,7 +27,7 @@ class FunctionIds(AbstractPrinter):
table = PrettyTable(['Name', 'ID'])
for function in contract.functions:
if function.visibility in ['public', 'external']:
table.add_row([function.full_name, hex(get_function_id(function.full_name))])
table.add_row([function.solidity_signature, hex(get_function_id(function.solidity_signature))])
for variable in contract.state_variables:
if variable.visibility in ['public']:
sig = variable.function_name

@ -61,7 +61,7 @@
"ending_column": 2
}
},
"signature": "bad0(address)"
"signature": "bad0(ERC20Function)"
}
},
{
@ -138,15 +138,15 @@
"ending_column": 2
}
},
"signature": "bad0(address)"
"signature": "bad0(ERC20Function)"
}
}
}
}
],
"description": "ERC20TestBalance.bad0(address) (tests/incorrect_equality.sol#21-23) uses a dangerous strict equality:\n\t- require(bool)(erc.balanceOf(address(this)) == 10) (tests/incorrect_equality.sol#22)\n",
"markdown": "[ERC20TestBalance.bad0(address)](tests/incorrect_equality.sol#L21-L23) uses a dangerous strict equality:\n\t- [require(bool)(erc.balanceOf(address(this)) == 10)](tests/incorrect_equality.sol#L22)\n",
"id": "15f4a74f8bfef6d5ccb8053319e212576a8f453ca411bbde81fecac9dd9c7581",
"description": "ERC20TestBalance.bad0(ERC20Function) (tests/incorrect_equality.sol#21-23) uses a dangerous strict equality:\n\t- require(bool)(erc.balanceOf(address(this)) == 10) (tests/incorrect_equality.sol#22)\n",
"markdown": "[ERC20TestBalance.bad0(ERC20Function)](tests/incorrect_equality.sol#L21-L23) uses a dangerous strict equality:\n\t- [require(bool)(erc.balanceOf(address(this)) == 10)](tests/incorrect_equality.sol#L22)\n",
"id": "75aa0ac0f7038b6a92030dee5c4c8f4cc6ab3f491558e18c61b6db5fbbf971e4",
"check": "incorrect-equality",
"impact": "Medium",
"confidence": "High"
@ -209,7 +209,7 @@
"ending_column": 2
}
},
"signature": "bad1(address)"
"signature": "bad1(ERC20Variable)"
}
},
{
@ -286,15 +286,15 @@
"ending_column": 2
}
},
"signature": "bad1(address)"
"signature": "bad1(ERC20Variable)"
}
}
}
}
],
"description": "ERC20TestBalance.bad1(address) (tests/incorrect_equality.sol#25-27) uses a dangerous strict equality:\n\t- require(bool)(erc.balanceOf(msg.sender) == 10) (tests/incorrect_equality.sol#26)\n",
"markdown": "[ERC20TestBalance.bad1(address)](tests/incorrect_equality.sol#L25-L27) uses a dangerous strict equality:\n\t- [require(bool)(erc.balanceOf(msg.sender) == 10)](tests/incorrect_equality.sol#L26)\n",
"id": "0f24486a6a14e20f6afccb0450cfcd9580308cef29fe011e05fb6a83aaa24da5",
"description": "ERC20TestBalance.bad1(ERC20Variable) (tests/incorrect_equality.sol#25-27) uses a dangerous strict equality:\n\t- require(bool)(erc.balanceOf(msg.sender) == 10) (tests/incorrect_equality.sol#26)\n",
"markdown": "[ERC20TestBalance.bad1(ERC20Variable)](tests/incorrect_equality.sol#L25-L27) uses a dangerous strict equality:\n\t- [require(bool)(erc.balanceOf(msg.sender) == 10)](tests/incorrect_equality.sol#L26)\n",
"id": "747d47c020b94e00fa06cc310b205306c37fda3811bafde5ee820ff84656127e",
"check": "incorrect-equality",
"impact": "Medium",
"confidence": "High"

@ -1,7 +1,7 @@

ERC20TestBalance.bad0(address) (tests/incorrect_equality.sol#21-23) uses a dangerous strict equality:
ERC20TestBalance.bad0(ERC20Function) (tests/incorrect_equality.sol#21-23) uses a dangerous strict equality:
- require(bool)(erc.balanceOf(address(this)) == 10) (tests/incorrect_equality.sol#22)
ERC20TestBalance.bad1(address) (tests/incorrect_equality.sol#25-27) uses a dangerous strict equality:
ERC20TestBalance.bad1(ERC20Variable) (tests/incorrect_equality.sol#25-27) uses a dangerous strict equality:
- require(bool)(erc.balanceOf(msg.sender) == 10) (tests/incorrect_equality.sol#26)
TestContractBalance.bad0() (tests/incorrect_equality.sol#32-35) uses a dangerous strict equality:
- require(bool)(address(address(this)).balance == 10000000000000000000) (tests/incorrect_equality.sol#33)

@ -57,7 +57,7 @@
"ending_column": 2
}
},
"signature": "bug(address)"
"signature": "bug(C)"
}
},
{
@ -130,7 +130,7 @@
"ending_column": 2
}
},
"signature": "bug(address)"
"signature": "bug(C)"
}
}
},
@ -208,7 +208,7 @@
"ending_column": 2
}
},
"signature": "bug(address)"
"signature": "bug(C)"
}
}
},
@ -217,9 +217,9 @@
}
}
],
"description": "Reentrancy in Test.bug(address) (tests/reentrancy-0.5.1-events.sol#14-17):\n\tExternal calls:\n\t- c.f() (tests/reentrancy-0.5.1-events.sol#15)\n\tEvent emitted after the call(s):\n\t- E() (tests/reentrancy-0.5.1-events.sol#16)\n",
"markdown": "Reentrancy in [Test.bug(address)](tests/reentrancy-0.5.1-events.sol#L14-L17):\n\tExternal calls:\n\t- [c.f()](tests/reentrancy-0.5.1-events.sol#L15)\n\tEvent emitted after the call(s):\n\t- [E()](tests/reentrancy-0.5.1-events.sol#L16)\n",
"id": "379907e63f185d72b3e767e005ba76247024692c633a93d4415c0a0be4ec1d8d",
"description": "Reentrancy in Test.bug(C) (tests/reentrancy-0.5.1-events.sol#14-17):\n\tExternal calls:\n\t- c.f() (tests/reentrancy-0.5.1-events.sol#15)\n\tEvent emitted after the call(s):\n\t- E() (tests/reentrancy-0.5.1-events.sol#16)\n",
"markdown": "Reentrancy in [Test.bug(C)](tests/reentrancy-0.5.1-events.sol#L14-L17):\n\tExternal calls:\n\t- [c.f()](tests/reentrancy-0.5.1-events.sol#L15)\n\tEvent emitted after the call(s):\n\t- [E()](tests/reentrancy-0.5.1-events.sol#L16)\n",
"id": "9654da7d8b8d85c90bc2ee1ddaea365f98f14d9981149b354f8a3d84f98ea576",
"check": "reentrancy-events",
"impact": "Low",
"confidence": "Medium"

@ -1,5 +1,5 @@

Reentrancy in Test.bug(address) (tests/reentrancy-0.5.1-events.sol#14-17):
Reentrancy in Test.bug(C) (tests/reentrancy-0.5.1-events.sol#14-17):
External calls:
- c.f() (tests/reentrancy-0.5.1-events.sol#15)
Event emitted after the call(s):

@ -70,7 +70,7 @@
"ending_column": 2
}
},
"signature": "test(address)"
"signature": "test(Target)"
}
},
{
@ -156,15 +156,15 @@
"ending_column": 2
}
},
"signature": "test(address)"
"signature": "test(Target)"
}
}
}
}
],
"description": "User.test(address) (tests/unused_return.sol#17-29) ignores return value by t.f() (tests/unused_return.sol#18)\n",
"markdown": "[User.test(address)](tests/unused_return.sol#L17-L29) ignores return value by [t.f()](tests/unused_return.sol#L18)\n",
"id": "accd4d71c13bd3ecae16bfa554bf755bf5a8923080e640089970e00ead85d51c",
"description": "User.test(Target) (tests/unused_return.sol#17-29) ignores return value by t.f() (tests/unused_return.sol#18)\n",
"markdown": "[User.test(Target)](tests/unused_return.sol#L17-L29) ignores return value by [t.f()](tests/unused_return.sol#L18)\n",
"id": "69f2810e24dbba754b406ce8b47e37543e9e491c0aa60d4dd2198c960e82b096",
"check": "unused-return",
"impact": "Medium",
"confidence": "Medium"
@ -236,7 +236,7 @@
"ending_column": 2
}
},
"signature": "test(address)"
"signature": "test(Target)"
}
},
{
@ -322,15 +322,15 @@
"ending_column": 2
}
},
"signature": "test(address)"
"signature": "test(Target)"
}
}
}
}
],
"description": "User.test(address) (tests/unused_return.sol#17-29) ignores return value by a.add(0) (tests/unused_return.sol#22)\n",
"markdown": "[User.test(address)](tests/unused_return.sol#L17-L29) ignores return value by [a.add(0)](tests/unused_return.sol#L22)\n",
"id": "78b4f6169d988d70d342626d7bc77ba6e04f9f256a2379949de6f5081d72c752",
"description": "User.test(Target) (tests/unused_return.sol#17-29) ignores return value by a.add(0) (tests/unused_return.sol#22)\n",
"markdown": "[User.test(Target)](tests/unused_return.sol#L17-L29) ignores return value by [a.add(0)](tests/unused_return.sol#L22)\n",
"id": "502f40d2e259e5e0268547489b716077dff7ce3df82fb05eb76ccb5ffa38f72b",
"check": "unused-return",
"impact": "Medium",
"confidence": "Medium"

@ -1,6 +1,6 @@

User.test(address) (tests/unused_return.sol#17-29) ignores return value by t.f() (tests/unused_return.sol#18)
User.test(address) (tests/unused_return.sol#17-29) ignores return value by a.add(0) (tests/unused_return.sol#22)
User.test(Target) (tests/unused_return.sol#17-29) ignores return value by t.f() (tests/unused_return.sol#18)
User.test(Target) (tests/unused_return.sol#17-29) ignores return value by a.add(0) (tests/unused_return.sol#22)
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#unused-return
tests/unused_return.sol analyzed (3 contracts with 1 detectors), 2 result(s) found
Use https://crytic.io/ to get access to additional detectors and Github integration

@ -4,11 +4,11 @@ Target functions:
The following functions reach the specified targets:
- A.call()
- B.call2(address)
- B.call2(A)
The following paths reach the specified targets:
A.call() -> A.destination()
B.call2(address) -> A.call() -> A.destination()
B.call2(A) -> A.call() -> A.destination()

Loading…
Cancel
Save