diff --git a/slither/detectors/statements/msg_value_in_loop.py b/slither/detectors/statements/msg_value_in_loop.py index 83c5658ca4..290447aa8e 100644 --- a/slither/detectors/statements/msg_value_in_loop.py +++ b/slither/detectors/statements/msg_value_in_loop.py @@ -8,6 +8,9 @@ from slither.slithir.operations import InternalCall from slither.core.declarations import SolidityVariableComposed, Contract from slither.utils.output import Output +from slither.slithir.variables.constant import Constant +from slither.core.variables import Variable +from slither.core.expressions.literal import Literal def detect_msg_value_in_loop(contract: Contract) -> List[Node]: @@ -37,6 +40,21 @@ def msg_value_in_loop( for ir in node.all_slithir_operations(): if in_loop_counter > 0 and SolidityVariableComposed("msg.value") in ir.read: + # If we find a conditional expression with msg.value and is compared to 0 we don't report it + if ir.node.is_conditional() and SolidityVariableComposed("msg.value") in ir.read: + compared_to = ( + ir.read[1] + if ir.read[0] == SolidityVariableComposed("msg.value") + else ir.read[0] + ) + if ( + isinstance(compared_to, Constant) + and compared_to.value == 0 + or isinstance(compared_to, Variable) + and isinstance(compared_to.expression, Literal) + and str(compared_to.expression.value) == "0" + ): + continue results.append(ir.node) if isinstance(ir, (InternalCall)): msg_value_in_loop(ir.function.entry_point, in_loop_counter, visited, results) diff --git a/tests/e2e/detectors/test_data/msg-value-loop/0.4.25/msg_value_loop.sol b/tests/e2e/detectors/test_data/msg-value-loop/0.4.25/msg_value_loop.sol index a32b79d8dc..e0f35861af 100644 --- a/tests/e2e/detectors/test_data/msg-value-loop/0.4.25/msg_value_loop.sol +++ b/tests/e2e/detectors/test_data/msg-value-loop/0.4.25/msg_value_loop.sol @@ -26,4 +26,38 @@ contract C{ } } + function good1(address[] memory receivers) public payable { + require(msg.value == 0); + for (uint256 i = 0; i < receivers.length; i++) { + balances[receivers[i]] += 1; + } + } + + function good2(address[] memory receivers) public payable { + uint zero = 0; + for (uint256 i = 0; i < receivers.length; i++) { + assert(msg.value == zero); + balances[receivers[i]] += 1; + } + } + + function good3(address[] memory receivers) public payable { + for (uint256 i = 0; i < receivers.length; i++) { + if (0 != msg.value) { + revert(); + } + balances[receivers[i]] += 1; + } + } + + function good4(address[] memory receivers) public payable { + for (uint256 i = 0; i < receivers.length; i++) { + _g(); + balances[receivers[i]] += 1; + } + } + + function _g() internal { + require(msg.value == 0); + } } \ No newline at end of file diff --git a/tests/e2e/detectors/test_data/msg-value-loop/0.4.25/msg_value_loop.sol-0.4.25.zip b/tests/e2e/detectors/test_data/msg-value-loop/0.4.25/msg_value_loop.sol-0.4.25.zip index 377d24cfd8..afa475f854 100644 Binary files a/tests/e2e/detectors/test_data/msg-value-loop/0.4.25/msg_value_loop.sol-0.4.25.zip and b/tests/e2e/detectors/test_data/msg-value-loop/0.4.25/msg_value_loop.sol-0.4.25.zip differ diff --git a/tests/e2e/detectors/test_data/msg-value-loop/0.5.16/msg_value_loop.sol b/tests/e2e/detectors/test_data/msg-value-loop/0.5.16/msg_value_loop.sol index a32b79d8dc..e0f35861af 100644 --- a/tests/e2e/detectors/test_data/msg-value-loop/0.5.16/msg_value_loop.sol +++ b/tests/e2e/detectors/test_data/msg-value-loop/0.5.16/msg_value_loop.sol @@ -26,4 +26,38 @@ contract C{ } } + function good1(address[] memory receivers) public payable { + require(msg.value == 0); + for (uint256 i = 0; i < receivers.length; i++) { + balances[receivers[i]] += 1; + } + } + + function good2(address[] memory receivers) public payable { + uint zero = 0; + for (uint256 i = 0; i < receivers.length; i++) { + assert(msg.value == zero); + balances[receivers[i]] += 1; + } + } + + function good3(address[] memory receivers) public payable { + for (uint256 i = 0; i < receivers.length; i++) { + if (0 != msg.value) { + revert(); + } + balances[receivers[i]] += 1; + } + } + + function good4(address[] memory receivers) public payable { + for (uint256 i = 0; i < receivers.length; i++) { + _g(); + balances[receivers[i]] += 1; + } + } + + function _g() internal { + require(msg.value == 0); + } } \ No newline at end of file diff --git a/tests/e2e/detectors/test_data/msg-value-loop/0.5.16/msg_value_loop.sol-0.5.16.zip b/tests/e2e/detectors/test_data/msg-value-loop/0.5.16/msg_value_loop.sol-0.5.16.zip index e6082623ef..2089eb4335 100644 Binary files a/tests/e2e/detectors/test_data/msg-value-loop/0.5.16/msg_value_loop.sol-0.5.16.zip and b/tests/e2e/detectors/test_data/msg-value-loop/0.5.16/msg_value_loop.sol-0.5.16.zip differ diff --git a/tests/e2e/detectors/test_data/msg-value-loop/0.6.11/msg_value_loop.sol b/tests/e2e/detectors/test_data/msg-value-loop/0.6.11/msg_value_loop.sol index a32b79d8dc..e0f35861af 100644 --- a/tests/e2e/detectors/test_data/msg-value-loop/0.6.11/msg_value_loop.sol +++ b/tests/e2e/detectors/test_data/msg-value-loop/0.6.11/msg_value_loop.sol @@ -26,4 +26,38 @@ contract C{ } } + function good1(address[] memory receivers) public payable { + require(msg.value == 0); + for (uint256 i = 0; i < receivers.length; i++) { + balances[receivers[i]] += 1; + } + } + + function good2(address[] memory receivers) public payable { + uint zero = 0; + for (uint256 i = 0; i < receivers.length; i++) { + assert(msg.value == zero); + balances[receivers[i]] += 1; + } + } + + function good3(address[] memory receivers) public payable { + for (uint256 i = 0; i < receivers.length; i++) { + if (0 != msg.value) { + revert(); + } + balances[receivers[i]] += 1; + } + } + + function good4(address[] memory receivers) public payable { + for (uint256 i = 0; i < receivers.length; i++) { + _g(); + balances[receivers[i]] += 1; + } + } + + function _g() internal { + require(msg.value == 0); + } } \ No newline at end of file diff --git a/tests/e2e/detectors/test_data/msg-value-loop/0.6.11/msg_value_loop.sol-0.6.11.zip b/tests/e2e/detectors/test_data/msg-value-loop/0.6.11/msg_value_loop.sol-0.6.11.zip index 2aeae50e86..fe0c5241cb 100644 Binary files a/tests/e2e/detectors/test_data/msg-value-loop/0.6.11/msg_value_loop.sol-0.6.11.zip and b/tests/e2e/detectors/test_data/msg-value-loop/0.6.11/msg_value_loop.sol-0.6.11.zip differ diff --git a/tests/e2e/detectors/test_data/msg-value-loop/0.7.6/msg_value_loop.sol b/tests/e2e/detectors/test_data/msg-value-loop/0.7.6/msg_value_loop.sol index a32b79d8dc..e0f35861af 100644 --- a/tests/e2e/detectors/test_data/msg-value-loop/0.7.6/msg_value_loop.sol +++ b/tests/e2e/detectors/test_data/msg-value-loop/0.7.6/msg_value_loop.sol @@ -26,4 +26,38 @@ contract C{ } } + function good1(address[] memory receivers) public payable { + require(msg.value == 0); + for (uint256 i = 0; i < receivers.length; i++) { + balances[receivers[i]] += 1; + } + } + + function good2(address[] memory receivers) public payable { + uint zero = 0; + for (uint256 i = 0; i < receivers.length; i++) { + assert(msg.value == zero); + balances[receivers[i]] += 1; + } + } + + function good3(address[] memory receivers) public payable { + for (uint256 i = 0; i < receivers.length; i++) { + if (0 != msg.value) { + revert(); + } + balances[receivers[i]] += 1; + } + } + + function good4(address[] memory receivers) public payable { + for (uint256 i = 0; i < receivers.length; i++) { + _g(); + balances[receivers[i]] += 1; + } + } + + function _g() internal { + require(msg.value == 0); + } } \ No newline at end of file diff --git a/tests/e2e/detectors/test_data/msg-value-loop/0.7.6/msg_value_loop.sol-0.7.6.zip b/tests/e2e/detectors/test_data/msg-value-loop/0.7.6/msg_value_loop.sol-0.7.6.zip index ef5fb240f1..577079412f 100644 Binary files a/tests/e2e/detectors/test_data/msg-value-loop/0.7.6/msg_value_loop.sol-0.7.6.zip and b/tests/e2e/detectors/test_data/msg-value-loop/0.7.6/msg_value_loop.sol-0.7.6.zip differ diff --git a/tests/e2e/detectors/test_data/msg-value-loop/0.8.0/msg_value_loop.sol b/tests/e2e/detectors/test_data/msg-value-loop/0.8.0/msg_value_loop.sol index a32b79d8dc..e0f35861af 100644 --- a/tests/e2e/detectors/test_data/msg-value-loop/0.8.0/msg_value_loop.sol +++ b/tests/e2e/detectors/test_data/msg-value-loop/0.8.0/msg_value_loop.sol @@ -26,4 +26,38 @@ contract C{ } } + function good1(address[] memory receivers) public payable { + require(msg.value == 0); + for (uint256 i = 0; i < receivers.length; i++) { + balances[receivers[i]] += 1; + } + } + + function good2(address[] memory receivers) public payable { + uint zero = 0; + for (uint256 i = 0; i < receivers.length; i++) { + assert(msg.value == zero); + balances[receivers[i]] += 1; + } + } + + function good3(address[] memory receivers) public payable { + for (uint256 i = 0; i < receivers.length; i++) { + if (0 != msg.value) { + revert(); + } + balances[receivers[i]] += 1; + } + } + + function good4(address[] memory receivers) public payable { + for (uint256 i = 0; i < receivers.length; i++) { + _g(); + balances[receivers[i]] += 1; + } + } + + function _g() internal { + require(msg.value == 0); + } } \ No newline at end of file diff --git a/tests/e2e/detectors/test_data/msg-value-loop/0.8.0/msg_value_loop.sol-0.8.0.zip b/tests/e2e/detectors/test_data/msg-value-loop/0.8.0/msg_value_loop.sol-0.8.0.zip index 13d17a8aa0..5eb9b1fceb 100644 Binary files a/tests/e2e/detectors/test_data/msg-value-loop/0.8.0/msg_value_loop.sol-0.8.0.zip and b/tests/e2e/detectors/test_data/msg-value-loop/0.8.0/msg_value_loop.sol-0.8.0.zip differ