From 68469facb0abfa1e6e6dd48a931e6eb3e9d96e20 Mon Sep 17 00:00:00 2001 From: Nikhil Parasaram Date: Thu, 9 Apr 2020 15:48:23 +0100 Subject: [PATCH] Update descriptions (#1359) --- .../module/modules/arbitrary_write.py | 4 +-- mythril/analysis/module/modules/exceptions.py | 2 +- mythril/analysis/module/modules/integer.py | 30 +++++++------------ .../analysis/module/modules/multiple_sends.py | 7 +++-- .../modules/state_change_external_calls.py | 10 ++++--- .../module/modules/unchecked_retval.py | 8 ++--- 6 files changed, 28 insertions(+), 33 deletions(-) diff --git a/mythril/analysis/module/modules/arbitrary_write.py b/mythril/analysis/module/modules/arbitrary_write.py index 601ad888..c6523213 100644 --- a/mythril/analysis/module/modules/arbitrary_write.py +++ b/mythril/analysis/module/modules/arbitrary_write.py @@ -64,10 +64,10 @@ class ArbitraryStorage(DetectionModule): function_name=state.environment.active_function_name, address=state.get_current_instruction()["address"], swc_id=WRITE_TO_ARBITRARY_STORAGE, - title="The caller can write to arbitrary storage locations.", + title="Write to an arbitrary storage location", severity="High", bytecode=state.environment.code.bytecode, - description_head="Any storage slot can be written by the caller.", + description_head="The caller can write to arbitrary storage locations.", description_tail="It is possible to write to arbitrary storage locations. By modifying the values of " "storage variables, attackers may bypass security controls or manipulate the business logic of " "the smart contract.", diff --git a/mythril/analysis/module/modules/exceptions.py b/mythril/analysis/module/modules/exceptions.py index 5d03501e..7a3a14f9 100644 --- a/mythril/analysis/module/modules/exceptions.py +++ b/mythril/analysis/module/modules/exceptions.py @@ -62,7 +62,7 @@ class Exceptions(DetectionModule): swc_id=ASSERT_VIOLATION, title="Exception State", severity="Medium", - description_head="An exception or assertion violation was triggered.", + description_head="An assertion violation was triggered.", description_tail=description_tail, bytecode=state.environment.code.bytecode, transaction_sequence=transaction_sequence, diff --git a/mythril/analysis/module/modules/integer.py b/mythril/analysis/module/modules/integer.py index e36cda12..d042061f 100644 --- a/mythril/analysis/module/modules/integer.py +++ b/mythril/analysis/module/modules/integer.py @@ -194,21 +194,6 @@ class IntegerArithmetics(DetectionModule): stack[index] = symbol_factory.BitVecVal(value, 256) return stack[index] - @staticmethod - def _get_description_head(annotation, _type): - return "The binary {} can {}.".format(annotation.operator, _type.lower()) - - @staticmethod - def _get_description_tail(annotation, _type): - - return ( - "It is possible to cause an integer {} in the {} operation. Prevent the {} by constraining inputs " - "using the require() statement or use the OpenZeppelin SafeMath library for integer arithmetic operations. " - "Refer to the transaction trace generated for this issue to reproduce the {}.".format( - _type.lower(), annotation.operator, _type.lower(), _type.lower() - ) - ) - @staticmethod def _get_title(_type): return "Integer {}".format(_type) @@ -313,17 +298,24 @@ class IntegerArithmetics(DetectionModule): except UnsatError: continue - _type = "Underflow" if annotation.operator == "subtraction" else "Overflow" + description_head = "The arithmetic operator can {}.".format( + "underflow" if annotation.operator == "subtraction" else "overflow" + ) + description_tail = "It is possible to cause an integer overflow or underflow in the arithmetic operation. " + "Prevent this by constraining inputs using the require() statement or use the OpenZeppelin " + "SafeMath library for integer arithmetic operations. " + "Refer to the transaction trace generated for this issue to reproduce the issue." + issue = Issue( contract=ostate.environment.active_account.contract_name, function_name=ostate.environment.active_function_name, address=ostate.get_current_instruction()["address"], swc_id=INTEGER_OVERFLOW_AND_UNDERFLOW, bytecode=ostate.environment.code.bytecode, - title=self._get_title(_type), + title="Integer Arithmetic Bugs", severity="High", - description_head=self._get_description_head(annotation, _type), - description_tail=self._get_description_tail(annotation, _type), + description_head=description_head, + description_tail=description_tail, gas_used=(state.mstate.min_gas_used, state.mstate.max_gas_used), transaction_sequence=transaction_sequence, ) diff --git a/mythril/analysis/module/modules/multiple_sends.py b/mythril/analysis/module/modules/multiple_sends.py index d0855d95..bc9e544d 100644 --- a/mythril/analysis/module/modules/multiple_sends.py +++ b/mythril/analysis/module/modules/multiple_sends.py @@ -76,9 +76,10 @@ class MultipleSends(DetectionModule): continue description_tail = ( "This call is executed following another call within the same transaction. It is possible " - "that the call never gets executed if a prior call fails permanently (this might be caused " - "intentionally by a malicious callee). If possible, refactor the code such that each transaction " - "only executes one external call." + "that the call never gets executed if a prior call fails permanently. This might be caused " + "intentionally by a malicious callee. If possible, refactor the code such that each transaction " + "only executes one external call or " + "make sure that all callees can be trusted (i.e. they’re part of your own codebase)." ) issue = Issue( diff --git a/mythril/analysis/module/modules/state_change_external_calls.py b/mythril/analysis/module/modules/state_change_external_calls.py index 1c024828..a508f0d4 100644 --- a/mythril/analysis/module/modules/state_change_external_calls.py +++ b/mythril/analysis/module/modules/state_change_external_calls.py @@ -77,10 +77,12 @@ class StateChangeCallsAnnotation(StateAnnotation): read_or_write ) description_tail = ( - "The contract account state is accessed after an external call to a {} address. Note that the callee " - "could re-enter any function in this contract before the state access has occurred. Review the contract " - "logic carefully and consider performing all state operations before executing the external call, " - "especially if the callee is not trusted.".format(address_type) + "The contract account state is accessed after an external call to a {} address. " + "To prevent reentrancy issues, consider accessing the state only before the call, especially if the callee is untrusted. " + "Alternatively, a reentrancy lock can be used to prevent " + "untrusted callees from re-entering the contract in an intermediate state.".format( + address_type + ) ) return PotentialIssue( diff --git a/mythril/analysis/module/modules/unchecked_retval.py b/mythril/analysis/module/modules/unchecked_retval.py index 0a22bdd7..fba9fbae 100644 --- a/mythril/analysis/module/modules/unchecked_retval.py +++ b/mythril/analysis/module/modules/unchecked_retval.py @@ -87,9 +87,9 @@ class UncheckedRetval(DetectionModule): description_tail = ( "External calls return a boolean value. If the callee halts with an exception, 'false' is " - "returned and execution continues in the caller. It is often desirable to wrap external calls " - "into a require() statement so the transaction is reverted if the call fails. Make sure that " - "no unexpected behaviour occurs if the call is unsuccessful." + "returned and execution continues in the caller. " + "The caller should check whether an exception happened and react accordingly to avoid unexpected behavior. " + "For example it is often desirable to wrap external calls in require() so the transaction is reverted if the call fails." ) issue = Issue( @@ -99,7 +99,7 @@ class UncheckedRetval(DetectionModule): bytecode=state.environment.code.bytecode, title="Unchecked return value from external call.", swc_id=UNCHECKED_RET_VAL, - severity="Low", + severity="Medium", description_head="The return value of a message call is not checked.", description_tail=description_tail, gas_used=(state.mstate.min_gas_used, state.mstate.max_gas_used),