@ -50,37 +50,13 @@ def is_assert(node):
return False return False
class DivideBeforeMultiply ( AbstractDetector ) : def _explore ( to_explore , f_results , divisions ) : # pylint: disable=too-many-branches
""" explored = set ( )
Divide before multiply while to_explore : # pylint: disable=too-many-nested-blocks
""" node = to_explore . pop ( )
ARGUMENT = " divide-before-multiply "
HELP = " Imprecise arithmetic operations order "
IMPACT = DetectorClassification . MEDIUM
CONFIDENCE = DetectorClassification . MEDIUM
WIKI = " https://github.com/crytic/slither/wiki/Detector-Documentation#divide-before-multiply "
WIKI_TITLE = " Divide before multiply "
WIKI_DESCRIPTION = """ Solidity integer division might truncate. As a result, performing multiplication before division can sometimes avoid loss of precision. """
WIKI_EXPLOIT_SCENARIO = """
` ` ` solidity
contract A {
function f ( uint n ) public {
coins = ( oldSupply / n ) * interest ;
}
}
` ` `
If ` n ` is greater than ` oldSupply ` , ` coins ` will be zero . For example , with ` oldSupply = 5 ; n = 10 , interest = 2 ` , coins will be zero .
If ` ( oldSupply * interest / n ) ` was used , ` coins ` would have been ` 1 ` .
In general , it ' s usually a good idea to re-arrange arithmetic to perform multiplication before division, unless the limit of a smaller type makes this dangerous. " " "
WIKI_RECOMMENDATION = """ Consider ordering multiplication before division. """
def _explore ( self , node , explored , f_results , divisions ) : # pylint: disable=too-many-branches
if node in explored : if node in explored :
return continue
explored . add ( node ) explored . add ( node )
equality_found = False equality_found = False
@ -121,47 +97,75 @@ In general, it's usually a good idea to re-arrange arithmetic to perform multipl
# We do not track the case where the multiplication is done in a require() or assert() # We do not track the case where the multiplication is done in a require() or assert()
# Which also contains a ==, to prevent FP due to the form # Which also contains a ==, to prevent FP due to the form
# assert(a == b * c + a % b) # assert(a == b * c + a % b)
if is_assert ( node ) and equality_found : if not ( is_assert ( node ) and equality_found ) :
pass
else :
f_results . append ( node_results ) f_results . append ( node_results )
for son in node . sons : for son in node . sons :
self . _explore ( son , explored , f_results , divisions ) to_explore . add ( son )
def detect_divide_before_multiply ( self , contract ) :
"""
Detects and returns all nodes with multiplications of division results .
: param contract : Contract to detect assignment within .
: return : A list of nodes with multiplications of divisions .
"""
# Create our result set. def detect_divide_before_multiply ( contract ) :
# List of tuple (function -> list(list(nodes))) """
# Each list(nodes) of the list is one bug instances Detects and returns all nodes with multiplications of division results .
# Each node in the list(nodes) is involved in the bug : param contract : Contract to detect assignment within .
results = [ ] : return : A list of nodes with multiplications of divisions .
"""
# Loop for each function and modifier. # Create our result set.
for function in contract . functions_declared : # List of tuple (function -> list(list(nodes)))
if not function . entry_point : # Each list(nodes) of the list is one bug instances
continue # Each node in the list(nodes) is involved in the bug
results = [ ]
# List of list(nodes) # Loop for each function and modifier.
# Each list(nodes) is one bug instances for function in contract . functions_declared :
f_results = [ ] if not function . entry_point :
continue
# lvalue -> node # List of list(nodes)
# track all the division results (and the assignment of the division results) # Each list(nodes) is one bug instances
divisions = defaultdict ( list ) f_results = [ ]
self . _explore ( function . entry_point , set ( ) , f_results , divisions ) # lvalue -> node
# track all the division results (and the assignment of the division results)
divisions = defaultdict ( list )
for f_result in f_results : _explore ( { function . entry_point } , f_results , divisions )
results . append ( ( function , f_result ) )
# Return the resulting set of nodes with divisions before multiplications for f_result in f_results :
return results results . append ( ( function , f_result ) )
# Return the resulting set of nodes with divisions before multiplications
return results
class DivideBeforeMultiply ( AbstractDetector ) :
"""
Divide before multiply
"""
ARGUMENT = " divide-before-multiply "
HELP = " Imprecise arithmetic operations order "
IMPACT = DetectorClassification . MEDIUM
CONFIDENCE = DetectorClassification . MEDIUM
WIKI = " https://github.com/crytic/slither/wiki/Detector-Documentation#divide-before-multiply "
WIKI_TITLE = " Divide before multiply "
WIKI_DESCRIPTION = """ Solidity integer division might truncate. As a result, performing multiplication before division can sometimes avoid loss of precision. """
WIKI_EXPLOIT_SCENARIO = """
` ` ` solidity
contract A {
function f ( uint n ) public {
coins = ( oldSupply / n ) * interest ;
}
}
` ` `
If ` n ` is greater than ` oldSupply ` , ` coins ` will be zero . For example , with ` oldSupply = 5 ; n = 10 , interest = 2 ` , coins will be zero .
If ` ( oldSupply * interest / n ) ` was used , ` coins ` would have been ` 1 ` .
In general , it ' s usually a good idea to re-arrange arithmetic to perform multiplication before division, unless the limit of a smaller type makes this dangerous. " " "
WIKI_RECOMMENDATION = """ Consider ordering multiplication before division. """
def _detect ( self ) : def _detect ( self ) :
""" """
@ -169,7 +173,7 @@ In general, it's usually a good idea to re-arrange arithmetic to perform multipl
""" """
results = [ ] results = [ ]
for contract in self . contracts : for contract in self . contracts :
divisions_before_multiplications = self . detect_divide_before_multiply ( contract ) divisions_before_multiplications = detect_divide_before_multiply ( contract )
if divisions_before_multiplications : if divisions_before_multiplications :
for ( func , nodes ) in divisions_before_multiplications : for ( func , nodes ) in divisions_before_multiplications :
@ -178,6 +182,9 @@ In general, it's usually a good idea to re-arrange arithmetic to perform multipl
" performs a multiplication on the result of a division: \n " , " performs a multiplication on the result of a division: \n " ,
] ]
# sort the nodes to get deterministic results
nodes . sort ( key = lambda x : x . node_id )
for node in nodes : for node in nodes :
info + = [ " \t - " , node , " \n " ] info + = [ " \t - " , node , " \n " ]
Josselin
4 years ago