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.

[sidarth16]

Hi @smonicas and @ep0chzer0
I’ve now added E2E test cases for this detector under:

tests/e2e/detectors/test_data/msg-value-in-nonpayable/
├── 0.4.25/
│   └── msg_value_in_nonpayable.sol
└── 0.8.0/
    └── msg_value_in_nonpayable.sol

Could you please guide me on the expected next steps to integrate these tests correctly?
In particular, should the detector tests include pre-generated .zip artifacts per Solidity version, or is there a standard workflow/script to generate them?

Thanks!

[ep0chzer0]

Hi @sidarth16, thanks for addressing the feedback - the changes look good!

Regarding the test setup: yes, you'll need to generate the snapshot files. The process is:

1. Add your test entry to tests/e2e/detectors/test_detectors.py
2. Generate the compiled artifacts (.zip files) using crytic-compile
3. Run the test once to generate the snapshot file, or create it manually with the expected output

For the zip artifacts, you can use Docker if you don't have the right solc version:

docker run --rm -v $(pwd):/src -w /src --platform linux/amd64 ghcr.io/crytic/crytic-compile:latest crytic-compile tests/e2e/detectors/test_data/msg-value-in-nonpayable/0.8.0/msg_value_in_nonpayable.sol --export-zip msg_value_in_nonpayable.sol-0.8.0.zip

---

Note to maintainers (@elopez, @smonicas): I also submitted PR #2923 for the same issue (#2781) before noticing sidarth16 had already addressed my review feedback here. Happy to close #2923 if you'd prefer to proceed with this PR - just let me know what works best for the project.