Add detector for msg.value usage unreachable from payable entry points

[sidarth16]

Summary

This PR resolves #2781 and introduces a new Slither detector, msg-value-in-nonpayable, which identifies uses of msg.value in functions that can never be reached from any payable public or external entry point.

In such cases, msg.value is provably meaningless:

• it is always zero, or
• execution always reverts .

This detector helps developers catch logic errors, dead code, and incorrect assumptions about ETH flow early.

Detection Logic

The detector:

• Identifies functions that directly read msg.value (including via modifiers)
• Builds reachability information using Slither’s call graph
• Checks whether the function can be reached from any payable public or external entry point
• Reports the function only if no payable entry point can reach it
• Provides call-chain context for non-payable entry points that can reach the function
  Only the function that uses msg.value is reported; callers are included for explanatory context.

Example

contract Subscription {
    mapping(address => bool) public subscribed;

    function subscribe() external {
        require(msg.value >= 1 ether, "Subscription fee required");
        subscribed[msg.sender] = true;
    }
}

Here, subscribe() is not payable and cannot receive ETH.
As a result, msg.value is always zero and the require statement always fails.

Notes

The detector follows the semantics discussed in #2781 and reports only msg.value usages that are provably unreachable from any payable execution path.

[sidarth16]

Detector output when executed on the TestCases contract provided in issue #2781 :

slither testcases.sol --detect msg-value-in-nonpayable

Flagged functions (as expected from #2781):

• test1()
• test2()
• test3()
• helper()

[ep0chzer0]

Review: Implementation Feedback

Thanks for working on this, the core detection logic is correct! However, I found a few issues that need to be addressed:

1. Output Format Issue (Critical)

The _build_info method uses strings instead of Slither objects, which breaks the JSON/SARIF output format:

Current (incorrect):

info = [
    "msg.value used in non-payable context\n",
    f"  Location: {func.source_mapping.filename.short}:{func.source_mapping.lines[0]}\n",
    ...
]

Expected (per Slither conventions):

info: DETECTOR_INFO = [
    func,
    " uses msg.value but is not reachable from any payable function\n",
]
if non_payable_callers:
    info.append("\tNon-payable entry points that can reach this function:\n")
    for caller in non_payable_callers:
        if caller != func:
            info.extend(["\t- ", caller, "\n"])

The Output class expects Slither objects (Function, Node, etc.) which it converts to proper source mappings for IDE integration and SARIF output.

2. Missing Import

You need to import DETECTOR_INFO from the abstract detector:

from slither.detectors.abstract_detector import (
    AbstractDetector,
    DetectorClassification,
    DETECTOR_INFO,
)

3. Include the Function Itself as Entry Point

In _get_entry_point_callers, you should include the function itself if it's public/external:

if function.visibility in ("public", "external"):
    if function.payable:
        payable_callers.append(function)
    else:
        non_payable_callers.append(function)

4. Missing is_implemented Check

Add a check to skip functions that aren't implemented:

if not func.is_implemented:
    continue

5. Missing Test Cases

The PR needs test data. Note that in Solidity 0.8+, the compiler prevents direct msg.value use in non-payable public/external functions, so tests should focus on internal functions that use msg.value but are only reachable from non-payable entry points.

Happy to help with any questions!

[sidarth16]

Hi @ep0chzer0 ,
thanks for the review.

I’ve now addressed the points you raised:

• Output format has been updated to use DETECTOR_INFO with Slither objects (e.g. Function) instead of raw strings.
• Added the is_implemented check to skip interface / abstract functions.
• Updated _get_entry_point_callers to include the function itself when it is public or external, to preserve correct entry-point semantics.
  • To avoid noisy output, the trivial self-edge is filtered only at presentation time, so it does not produce empty or redundant call-chain entries.

Here is the updated detector output now when running on the TestCases contract from #2781:

INFO:Detectors:
Detector: msg-value-in-nonpayable
TestCases.test1() (testcases.sol#11-13) uses msg.value but is not reachable from any payable function
TestCases.test2() (testcases.sol#16-18) uses msg.value but is not reachable from any payable function
TestCases.test3() (testcases.sol#21-23) uses msg.value but is not reachable from any payable function
TestCases.helper() (testcases.sol#40-42) uses msg.value but is not reachable from any payable function
        Non-payable entry points that can reach this function:
        - TestCases.entry() (testcases.sol#48-50)

Reference: https://github.com/crytic/slither/issues/2781
INFO:Slither:testcases.sol analyzed (1 contracts with 1 detectors), 4 result(s) found

I will also be adding the TestCases.sol contract referenced in #2781 to the test suite.
This contract is useful as it covers both:

• Solidity 0.4.x semantics (where msg.value in non-payable functions is allowed but semantically incorrect), and
• Solidity ≥0.8.x-compatible scenarios via internal functions reachable only from non-payable entry points.