Merge pull request #2488 from crytic/dev-arb-send-eth-immutable

fix(arbitrary-send-eth): don't report if destination is immutable state var

slither/detectors/functions/arbitrary_send_eth.py

@@ -30,6 +30,7 @@ from slither.slithir.operations import (
     SolidityCall,
     Transfer,
 )
+from slither.core.variables.state_variable import StateVariable
 
 # pylint: disable=too-many-nested-blocks,too-many-branches
 from slither.utils.output import Output
@@ -67,6 +68,8 @@ def arbitrary_send(func: Function) -> Union[bool, List[Node]]:
                     continue
                 if ir.call_value == SolidityVariableComposed("msg.value"):
                     continue
+                if isinstance(ir.destination, StateVariable) and ir.destination.is_immutable:
+                    continue
                 if is_dependent(
                     ir.call_value,
                     SolidityVariableComposed("msg.value"),

tests/e2e/detectors/snapshots/detectors__detector_ArbitrarySendEth_0_6_11_arbitrary_send_eth_sol__0.txt

@@ -1,8 +1,8 @@
-Test.indirect() (tests/e2e/detectors/test_data/arbitrary-send-eth/0.6.11/arbitrary_send_eth.sol#19-21) sends eth to arbitrary user
+Test.direct() (tests/e2e/detectors/test_data/arbitrary-send-eth/0.6.11/arbitrary_send_eth.sol#16-18) sends eth to arbitrary user
 	Dangerous calls:
-	- destination.send(address(this).balance) (tests/e2e/detectors/test_data/arbitrary-send-eth/0.6.11/arbitrary_send_eth.sol#20)
+	- msg.sender.send(address(this).balance) (tests/e2e/detectors/test_data/arbitrary-send-eth/0.6.11/arbitrary_send_eth.sol#17)
 
-Test.direct() (tests/e2e/detectors/test_data/arbitrary-send-eth/0.6.11/arbitrary_send_eth.sol#11-13) sends eth to arbitrary user
+Test.indirect() (tests/e2e/detectors/test_data/arbitrary-send-eth/0.6.11/arbitrary_send_eth.sol#24-26) sends eth to arbitrary user
 	Dangerous calls:
-	- msg.sender.send(address(this).balance) (tests/e2e/detectors/test_data/arbitrary-send-eth/0.6.11/arbitrary_send_eth.sol#12)
+	- destination.send(address(this).balance) (tests/e2e/detectors/test_data/arbitrary-send-eth/0.6.11/arbitrary_send_eth.sol#25)
 

tests/e2e/detectors/snapshots/detectors__detector_ArbitrarySendEth_0_7_6_arbitrary_send_eth_sol__0.txt

@@ -1,8 +1,8 @@
-Test.direct() (tests/e2e/detectors/test_data/arbitrary-send-eth/0.7.6/arbitrary_send_eth.sol#11-13) sends eth to arbitrary user
+Test.direct() (tests/e2e/detectors/test_data/arbitrary-send-eth/0.7.6/arbitrary_send_eth.sol#16-18) sends eth to arbitrary user
 	Dangerous calls:
-	- msg.sender.send(address(this).balance) (tests/e2e/detectors/test_data/arbitrary-send-eth/0.7.6/arbitrary_send_eth.sol#12)
+	- msg.sender.send(address(this).balance) (tests/e2e/detectors/test_data/arbitrary-send-eth/0.7.6/arbitrary_send_eth.sol#17)
 
-Test.indirect() (tests/e2e/detectors/test_data/arbitrary-send-eth/0.7.6/arbitrary_send_eth.sol#19-21) sends eth to arbitrary user
+Test.indirect() (tests/e2e/detectors/test_data/arbitrary-send-eth/0.7.6/arbitrary_send_eth.sol#24-26) sends eth to arbitrary user
 	Dangerous calls:
-	- destination.send(address(this).balance) (tests/e2e/detectors/test_data/arbitrary-send-eth/0.7.6/arbitrary_send_eth.sol#20)
+	- destination.send(address(this).balance) (tests/e2e/detectors/test_data/arbitrary-send-eth/0.7.6/arbitrary_send_eth.sol#25)
 

tests/e2e/detectors/test_data/arbitrary-send-eth/0.6.11/arbitrary_send_eth.sol

@@ -1,13 +1,18 @@
 contract Test{
 
     address payable destination;
-
+    address payable immutable destination_imm;
     mapping (address => uint) balances;
 
     constructor() public{
+        destination_imm = payable(msg.sender);
         balances[msg.sender] = 0;
     }
 
+    function send_immutable() public{
+        destination_imm.send(address(this).balance);
+    }
+
     function direct() public{
         msg.sender.send(address(this).balance);
     }

tests/e2e/detectors/test_data/arbitrary-send-eth/0.7.6/arbitrary_send_eth.sol

@@ -1,13 +1,18 @@
 contract Test{
 
     address payable destination;
-
+    address payable immutable destination_imm;
     mapping (address => uint) balances;
 
     constructor() public{
+        destination_imm = payable(msg.sender);
         balances[msg.sender] = 0;
     }
 
+    function send_immutable() public{
+        destination_imm.send(address(this).balance);
+    }
+
     function direct() public{
         msg.sender.send(address(this).balance);
     }