Add return-bomb detector

slither/detectors/all_detectors.py

@@ -96,3 +96,4 @@
 from .assembly.return_instead_of_leave import ReturnInsteadOfLeave
 from .operations.incorrect_exp import IncorrectOperatorExponentiation
 from .statements.tautological_compare import TautologicalCompare
+from .statements.return_bomb import ReturnBomb

slither/detectors/statements/return_bomb.py

@@ -0,0 +1,123 @@
+from typing import List
+
+from slither.core.cfg.node import Node
+from slither.core.declarations import Contract
+from slither.core.declarations.function import Function
+from slither.core.solidity_types import Type
+from slither.detectors.abstract_detector import (
+    AbstractDetector,
+    DetectorClassification,
+    DETECTOR_INFO,
+)
+from slither.slithir.operations import LowLevelCall, HighLevelCall
+from slither.analyses.data_dependency.data_dependency import is_tainted
+from slither.utils.output import Output
+
+
+class ReturnBomb(AbstractDetector):
+
+    ARGUMENT = "return-bomb"
+    HELP = "A low level callee may consume all callers gas unexpectedly."
+    IMPACT = DetectorClassification.LOW
+    CONFIDENCE = DetectorClassification.MEDIUM
+
+    WIKI = "https://github.com/crytic/slither/wiki/Detector-Documentation#return-bomb"
+
+    WIKI_TITLE = "Return Bomb"
+    WIKI_DESCRIPTION = "A low level callee may consume all callers gas unexpectedly."
+    WIKI_EXPLOIT_SCENARIO = """
+```solidity
+//Modified from https://github.com/nomad-xyz/ExcessivelySafeCall
+contract BadGuy {
+    function youveActivateMyTrapCard() external pure returns (bytes memory) {
+        assembly{
+            revert(0, 1000000)
+        }
+    }
+}
+
+contract Mark {
+    function oops(address badGuy) public{
+        bool success;
+        bytes memory ret;
+
+        // Mark pays a lot of gas for this copy
+        //(success, ret) = badGuy.call{gas:10000}(
+        (success, ret) = badGuy.call(
+            abi.encodeWithSelector(
+                BadGuy.youveActivateMyTrapCard.selector
+            )
+        );
+
+        // Mark may OOG here, preventing local state changes
+        //importantCleanup();
+    }
+}
+
+```
+After Mark calls BadGuy bytes are copied from returndata to memory, the memory expansion cost is paid. This means that when using a standard solidity call, the callee can "returnbomb" the caller, imposing an arbitrary gas cost. 
+Callee unexpectedly makes the caller OOG. 
+"""
+
+    WIKI_RECOMMENDATION = "Avoid unlimited implicit decoding of returndata."
+
+    @staticmethod
+    def is_dynamic_type(ty: Type) -> bool:
+        # ty.is_dynamic ?
+        name = str(ty)
+        if "[]" in name or name in ("bytes", "string"):
+            return True
+        return False
+
+    def get_nodes_for_function(self, function: Function, contract: Contract) -> List[Node]:
+        nodes = []
+        for node in function.nodes:
+            for ir in node.irs:
+                if isinstance(ir, (HighLevelCall, LowLevelCall)):
+                    if not is_tainted(ir.destination, contract):  # type:ignore
+                        # Only interested if the target address is controlled/tainted
+                        continue
+
+                    if isinstance(ir, HighLevelCall) and isinstance(ir.function, Function):
+                        # in normal highlevel calls return bombs are _possible_
+                        # if the return type is dynamic and the caller tries to copy and decode large data
+                        has_dyn = False
+                        if ir.function.return_type:
+                            has_dyn = any(
+                                self.is_dynamic_type(ty) for ty in ir.function.return_type
+                            )
+
+                        if not has_dyn:
+                            continue
+
+                    # If a gas budget was specified then the
+                    # user may not know about the return bomb
+                    if ir.call_gas is None:
+                        # if a gas budget was NOT specified then the caller
+                        # may already suspect the call may spend all gas?
+                        continue
+
+                    nodes.append(node)
+                # TODO: check that there is some state change after the call
+
+        return nodes
+
+    def _detect(self) -> List[Output]:
+        results = []
+
+        for contract in self.compilation_unit.contracts:
+            for function in contract.functions_declared:
+                nodes = self.get_nodes_for_function(function, contract)
+                if nodes:
+                    info: DETECTOR_INFO = [
+                        function,
+                        " tries to limit the gas of an external call that controls implicit decoding\n",
+                    ]
+
+                    for node in sorted(nodes, key=lambda x: x.node_id):
+                        info += ["\t", node, "\n"]
+
+                    res = self.generate_result(info)
+                    results.append(res)
+
+        return results

tests/e2e/detectors/snapshots/detectors__detector_ReturnBomb_0_8_20_return_bomb_sol__0.txt

@@ -0,0 +1,5 @@
+Mark.oops(address) (tests/e2e/detectors/test_data/return-bomb/0.8.20/return_bomb.sol#31-55) tries to limit the gas of an external call that controls implicit decoding
+	ret1 = BadGuy(badGuy).fbad{gas: 2000}() (tests/e2e/detectors/test_data/return-bomb/0.8.20/return_bomb.sol#42)
+	(x,str) = BadGuy(badGuy).fbad1{gas: 2000}() (tests/e2e/detectors/test_data/return-bomb/0.8.20/return_bomb.sol#44)
+	(success,ret) = badGuy.call{gas: 10000}(abi.encodeWithSelector(BadGuy.llbad.selector)) (tests/e2e/detectors/test_data/return-bomb/0.8.20/return_bomb.sol#47-51)
+

tests/e2e/detectors/snapshots/detectors__detector_TautologicalCompare_0_8_20_compare_sol__0.txt

@@ -0,0 +1,3 @@
+A.check(uint256) (tests/e2e/detectors/test_data/tautological-compare/0.8.20/compare.sol#3-5) compares a variable to itself:
+	(a >= a) (tests/e2e/detectors/test_data/tautological-compare/0.8.20/compare.sol#4)
+

tests/e2e/detectors/test_data/return-bomb/0.8.20/return_bomb.sol

@@ -0,0 +1,57 @@
+contract BadGuy {
+    function llbad() external pure returns (bytes memory) {
+        assembly{
+            revert(0, 1000000)
+        }
+    }
+
+    function fgood() external payable returns (uint){
+        assembly{
+            return(0, 1000000)
+        }
+    }
+
+    function fbad() external payable returns (uint[] memory){
+        assembly{
+            return(0, 1000000)
+        }
+    }
+
+    function fbad1() external payable returns (uint, string memory){
+        assembly{
+            return(0, 1000000)
+        }
+    }
+
+
+}
+
+contract Mark {
+
+    function oops(address badGuy) public{
+        bool success;
+        string memory str;
+        bytes memory ret;
+        uint x;
+        uint[] memory ret1;
+
+	x = BadGuy(badGuy).fgood{gas:2000}();
+
+	ret1 = BadGuy(badGuy).fbad(); //good (no gas specified)
+
+	ret1 = BadGuy(badGuy).fbad{gas:2000}();
+
+	(x, str) = BadGuy(badGuy).fbad1{gas:2000}();
+
+        // Mark pays a lot of gas for this copy 😬😬😬
+        (success, ret) = badGuy.call{gas:10000}(
+            abi.encodeWithSelector(
+                BadGuy.llbad.selector
+            )
+        );
+
+        // Mark may OOG here, preventing local state changes
+        //importantCleanup();
+    }
+}
+

tests/e2e/detectors/test_detectors.py

@@ -1674,6 +1674,11 @@ def id_test(test_item: Test):
         "compare.sol",
         "0.8.20",
     ),
+    Test(
+        all_detectors.ReturnBomb,
+        "return_bomb.sol",
+        "0.8.20",
+    ),
 ]
 
 GENERIC_PATH = "/GENERIC_PATH"