Skip to content

A modifier that reads msg.value and writes state loses the state-write requirement: an internal view function using it compiles into code containing SSTORE #16980

Description

@Lokkw0510

Description

ViewPureChecker infers a modifier's state mutability as a single ordered maximum over
StateMutability { Pure, View, NonPayable, Payable }. Payable and NonPayable are not ordered
requirements though — they are independent obligations ("may receive value" vs "writes state").
When a modifier does both, Payable is numerically greater and the state write is silently
discarded.

Payable is then deliberately permitted on non-public functions, so an internal or private
function declared view receives no diagnostic at all and is compiled with the modifier's
SSTORE inlined into it.

The practical effect is that adding an unrelated msg.value read to a modifier suppresses an
error that solc otherwise reports
.

Environment

  • Compiler version: 0.8.36 (source of develop is identical at these lines)
  • Both pipelines (legacy and --via-ir)

Reproducer

// SPDX-License-Identifier: GPL-3.0
pragma solidity ^0.8.0;

contract C {
    uint public x;
    modifier m() { uint v = msg.value; v; x = 1; _; }
    function f() internal view m returns (uint) { return x; }
    function g() public   view    returns (uint) { return f(); }
}

Compiles with no error and no warning (solc 0.8.36, legacy and via-IR). solc --asm shows
sstore in the runtime code, and solc --abi reports "stateMutability": "view" for g.

Executed (py-evm, Shanghai):

x() -> 0
g() -> 1        // g is declared `public view`
x() -> 1        // ... and it wrote storage

Control

Delete uint v = msg.value; v; from the modifier and leave everything else identical:

Error 8961: Function cannot be declared as view because this expression (potentially) modifies the state.
  --> control.sol:10:32:
   |
10 |     function f() internal view m returns (uint) { return x; }
   |                                ^

Full matrix (solc 0.8.36), modifier = { uint v = msg.value; v; x = 1; }:

function declaration result
function f() internal view m accepted (bug)
function f() private view m accepted (bug)
function f() internal pure m rejected (2527) — caught only incidentally, by the Payable && Pure branch
function f() public view m rejected (4006), but the message is about msg.value and never mentions the state write

Cause

libsolidity/analysis/ViewPureChecker.cpp:239-240:

if (_mutability > m_bestMutabilityAndLocation.mutability)
    m_bestMutabilityAndLocation = MutabilityAndLocation{_mutability, _location};

While inferring the modifier, x = 1 reports NonPayable and msg.value reports Payable;
Payable > NonPayable (libsolidity/ast/ASTEnums.h:37), so only Payable survives.
modifierMutability() stores that single value, and endVisit(ModifierInvocation)
(:462-468) passes it to reportMutability, whose Payable branch (:270-295) errors only when
m_currentFunction->isConstructor() || m_currentFunction->isPublic() — intentionally, so internal
functions may use msg.value. The discarded NonPayable requirement is never reported by anyone.

A fix needs the payable requirement tracked separately from the view/pure requirement rather than
folded into one maximum.

Relationship to #16931

#16931 reports the same symptom class — a
pure/view function silently gaining state access through a modifier — but by a different
mechanism: overriding a virtual modifier, where resolveVirtual() re-resolves past the modifier
ViewPureChecker validated against. This report needs no inheritance, no virtual and no
override; the statically bound modifier's own inferred mutability is already wrong. A fix in
OverrideChecker for #16931 would not address this.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions