diff --git a/slither/detectors/all_detectors.py b/slither/detectors/all_detectors.py index 4a111bb64a..97622f8878 100644 --- a/slither/detectors/all_detectors.py +++ b/slither/detectors/all_detectors.py @@ -94,3 +94,5 @@ from .operations.encode_packed import EncodePackedCollision from .assembly.incorrect_return import IncorrectReturn from .assembly.return_instead_of_leave import ReturnInsteadOfLeave +from .operations.incorrect_exp import IncorrectOperatorExponentiation +from .statements.tautological_compare import TautologicalCompare diff --git a/slither/detectors/operations/incorrect_exp.py b/slither/detectors/operations/incorrect_exp.py index e69de29bb2..6188f5cb16 100644 --- a/slither/detectors/operations/incorrect_exp.py +++ b/slither/detectors/operations/incorrect_exp.py @@ -0,0 +1,93 @@ +""" +Module detecting incorrect operator usage for exponentiation where bitwise xor '^' is used instead of '**' +""" +from typing import Tuple, List, Union + +from slither.core.cfg.node import Node +from slither.core.declarations import Contract, Function +from slither.detectors.abstract_detector import ( + AbstractDetector, + DetectorClassification, + DETECTOR_INFO, +) +from slither.slithir.operations import Binary, BinaryType, Operation +from slither.slithir.utils.utils import RVALUE +from slither.slithir.variables.constant import Constant +from slither.utils.output import Output + + +def _is_constant_candidate(var: Union[RVALUE, Function]) -> bool: + """ + Check if the variable is a constant. + Do not consider variable that are expressed with hexadecimal. + Something like 2^0xf is likely to be a correct bitwise operator + :param var: + :return: + """ + return isinstance(var, Constant) and not var.original_value.startswith("0x") + + +def _is_bitwise_xor_on_constant(ir: Operation) -> bool: + return ( + isinstance(ir, Binary) + and ir.type == BinaryType.CARET + and (_is_constant_candidate(ir.variable_left) or _is_constant_candidate(ir.variable_right)) + ) + + +def _detect_incorrect_operator(contract: Contract) -> List[Tuple[Function, Node]]: + ret: List[Tuple[Function, Node]] = [] + f: Function + for f in contract.functions + contract.modifiers: # type:ignore + # Heuristic: look for binary expressions with ^ operator where at least one of the operands is a constant, and + # the constant is not in hex, because hex typically is used with bitwise xor and not exponentiation + nodes = [node for node in f.nodes for ir in node.irs if _is_bitwise_xor_on_constant(ir)] + for node in nodes: + ret.append((f, node)) + return ret + + +# pylint: disable=too-few-public-methods +class IncorrectOperatorExponentiation(AbstractDetector): + """ + Incorrect operator usage of bitwise xor mistaking it for exponentiation + """ + + ARGUMENT = "incorrect-exp" + HELP = "Incorrect exponentiation" + IMPACT = DetectorClassification.HIGH + CONFIDENCE = DetectorClassification.MEDIUM + + WIKI = "https://github.com/crytic/slither/wiki/Detector-Documentation#incorrect-exponentiation" + + WIKI_TITLE = "Incorrect exponentiation" + WIKI_DESCRIPTION = "Detect use of bitwise `xor ^` instead of exponential `**`" + WIKI_EXPLOIT_SCENARIO = """ +```solidity +contract Bug{ + uint UINT_MAX = 2^256 - 1; + ... +} +``` +Alice deploys a contract in which `UINT_MAX` incorrectly uses `^` operator instead of `**` for exponentiation""" + + WIKI_RECOMMENDATION = "Use the correct operator `**` for exponentiation." + + def _detect(self) -> List[Output]: + """Detect the incorrect operator usage for exponentiation where bitwise xor ^ is used instead of ** + + Returns: + list: (function, node) + """ + results: List[Output] = [] + for c in self.compilation_unit.contracts_derived: + res = _detect_incorrect_operator(c) + for (func, node) in res: + info: DETECTOR_INFO = [ + func, + " has bitwise-xor operator ^ instead of the exponentiation operator **: \n", + ] + info += ["\t - ", node, "\n"] + results.append(self.generate_result(info)) + + return results diff --git a/slither/detectors/statements/tautological_compare.py b/slither/detectors/statements/tautological_compare.py new file mode 100644 index 0000000000..0dc34b582f --- /dev/null +++ b/slither/detectors/statements/tautological_compare.py @@ -0,0 +1,69 @@ +from typing import List +from slither.detectors.abstract_detector import ( + AbstractDetector, + DetectorClassification, + DETECTOR_INFO, +) +from slither.slithir.operations import ( + Binary, + BinaryType, +) + +from slither.core.declarations import Function +from slither.utils.output import Output + + +class TautologicalCompare(AbstractDetector): + """ + Same variable comparison detector + """ + + ARGUMENT = "tautological-compare" + HELP = "Comparing a variable to itself always returns true or false, depending on comparison" + IMPACT = DetectorClassification.MEDIUM + CONFIDENCE = DetectorClassification.HIGH + + WIKI = "https://github.com/crytic/slither/wiki/Detector-Documentation#tautological-compare" + + WIKI_TITLE = "Tautological compare" + WIKI_DESCRIPTION = "A variable compared to itself is probably an error as it will always return `true` for `==`, `>=`, `<=` and always `false` for `<`, `>` and `!=`." + WIKI_EXPLOIT_SCENARIO = """ +```solidity + function check(uint a) external returns(bool){ + return (a >= a); + } +``` +`check` always return true.""" + + WIKI_RECOMMENDATION = "Remove comparison or compare to different value." + + def _check_function(self, f: Function) -> List[Output]: + affected_nodes = set() + for node in f.nodes: + for ir in node.irs: + if isinstance(ir, Binary): + if ir.type in [ + BinaryType.GREATER, + BinaryType.GREATER_EQUAL, + BinaryType.LESS, + BinaryType.LESS_EQUAL, + BinaryType.EQUAL, + BinaryType.NOT_EQUAL, + ]: + if ir.variable_left == ir.variable_right: + affected_nodes.add(node) + + results = [] + for n in affected_nodes: + info: DETECTOR_INFO = [f, " compares a variable to itself:\n\t", n, "\n"] + res = self.generate_result(info) + results.append(res) + return results + + def _detect(self): + results = [] + + for f in self.compilation_unit.functions_and_modifiers: + results.extend(self._check_function(f)) + + return results diff --git a/tests/e2e/detectors/test_data/tautological-compare/0.8.20/compare.sol b/tests/e2e/detectors/test_data/tautological-compare/0.8.20/compare.sol new file mode 100644 index 0000000000..ede2f0fc18 --- /dev/null +++ b/tests/e2e/detectors/test_data/tautological-compare/0.8.20/compare.sol @@ -0,0 +1,6 @@ + +contract A{ + function check(uint a) external returns(bool){ + return (a >= a); + } +} \ No newline at end of file diff --git a/tests/e2e/detectors/test_data/tautological-compare/0.8.20/compare.sol-0.8.20.zip b/tests/e2e/detectors/test_data/tautological-compare/0.8.20/compare.sol-0.8.20.zip new file mode 100644 index 0000000000..05b78fad08 Binary files /dev/null and b/tests/e2e/detectors/test_data/tautological-compare/0.8.20/compare.sol-0.8.20.zip differ diff --git a/tests/e2e/detectors/test_detectors.py b/tests/e2e/detectors/test_detectors.py index 9ca1bfcde9..99e9095fff 100644 --- a/tests/e2e/detectors/test_detectors.py +++ b/tests/e2e/detectors/test_detectors.py @@ -1664,6 +1664,16 @@ def id_test(test_item: Test): "incorrect_return.sol", "0.8.10", ), + Test( + all_detectors.IncorrectOperatorExponentiation, + "incorrect_exp.sol", + "0.7.6", + ), + Test( + all_detectors.TautologicalCompare, + "compare.sol", + "0.8.20", + ), ] GENERIC_PATH = "/GENERIC_PATH"