fix: F-2026-18144 | [Dual Defense] Unchecked Cumulative GasWanted Can Fail FinalizeBlock Under Unbounded Block Gas - #350
Merged
Merged
Conversation
…(F-2026-18144) Fee-paying txs are self-limiting: the ante handler requires ceil(minGasPrice * gasLimit), so an absurd gas limit costs absurd money. Gasless txs pay nothing, so nothing bounded the gas they declared, while that declared gas was still added to the fee market's cumulative gas wanted for the block. Under max_gas: -1 the per-tx block-limit check is inert, so two gasless txs each declaring MaxInt64 sum past what EndBlock can convert to int64 - an error that surfaces through FinalizeBlock after the block is decided. New uexecutor param max_gasless_tx_gas (default 100,000,000), enforced by GaslessGasLimitDecorator on the existing txpolicy.IsGaslessTx predicate, before NewGasWantedDecorator accumulates the declaration. A parameter and not a constant because "too low" stops the universal validators voting and must be fixable by proposal in minutes. Zero is rejected at genesis and on update, and an unreadable or unset parameter falls back to the default rather than to "no cap". universalClient/pushsigner declares 100,000,000 instead of 500,000,000. Live donut data: gas_wanted median/max 500,000,000 against a gas_used max of 5,011,877 - the largest real consumer used 1.0024% of what it declared.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue
Three individually-fine things collide:
Donut runs
max_gas: -1(in-treeupdate_max_block_gas.json). Two transactions each declaringMaxInt64sum into(MaxInt64, MaxUint64]— a validuint64that cannot convert toint64, withneither transaction individually rejectable. The error comes out of
FinalizeBlock, after theblock is decided: CometBFT at height H, application stuck at H−1.
Gasless transactions are the whole attack surface.
app/cosmos/min_gas_price.goderives therequired fee from the declared gas —
fee = ceil(minGasPrice * gasLimit)— so a fee-paying txdeclaring
MaxInt64owes roughly4.6e36 upcat donut's parameters and never gets past the ante.Fee-exempt messages pay nothing, so nothing bounds what they declare. The defence therefore has to
be a hard cap, not a price.
Live donut measurement, 400 blocks from height 22,076,325, 133 txs with
gas_wanted > 0:Every universal-validator vote arrives as an
authz.MsgExecdeclaring the hardcoded 500,000,000from
universalClient/pushsigner/vote.go.Change
A governance-controlled cap on the gas a fee-exempt transaction may declare.
max_gasless_tx_gasparam, default100,000,000proto/uexecutor/v1/types.proto,x/uexecutor/types/params.goGaslessGasLimitDecorator— rejects a gasless tx declaring moreapp/ante/gasless_gas_limit.goNewGasWantedDecorator, right afterSetUpContextapp/ante/ante_cosmos.goUexecutorKeeperonHandlerOptions(+Validate())app/ante/handler_options.go,app/app.goUpdateParamsvalidates before writing;GetParamsaccessorx/uexecutor/keeper/msg_update_params.gouniversalClient/pushsigner/vote.goWhy 100,000,000: ~20x the observed peak
gas_usedof 5,011,877, and a 5x reduction fromtoday's declaration. Overflow stays unreachable —
100M x ~1e5 txs/21MB block ~= 1e13againstMaxInt64 ~= 9.2e18, six orders of magnitude of headroom.Why a parameter and not a constant: the failure mode of "too low" is universal validators
unable to vote. That has to be fixable by proposal in minutes, not by a binary release.
Zero can never mean "no cap".
ValidateBasicrejectsmax_gasless_tx_gas == 0at genesis andon every
MsgUpdateParams, and the decorator falls back to the 100,000,000 default when theparameter is unreadable or unset. A missing parameter fails safe, not open.
Where it sits. The cap hangs off the existing
txpolicy.IsGaslessTx(tx)predicate — the sameone used by
app/ante/fee.go,app/cosmos/min_gas_price.goandapp/ante/account_init_decorator.go— so it inherits PR #332's empty-MsgExecfix and cannot bedodged by that vacuous-truth trick. It runs immediately after
SetUpContextDecoratorso the storeread is metered, and well before
NewGasWantedDecorator, which is what performs the accumulation.audit-fixesis fresh-genesis, so the new parameter needs no upgrade handler or state migration —it ships in
DefaultParams().Rollout ordering
The universal validators hardcoded 500,000,000. A 100,000,000 cap rejects every vote until the
fleet declares less, which is why the client constant moves in this same PR. Roll the universal
validator fleet before, or with, the chain — getting this backwards is a worse outage than the bug.
Tests
app/ante/gasless_gas_limit_test.go(unit, 9 tests):above/at/below the cap;
MaxInt64; a fee-paying tx withMaxInt64is not capped (and the paramsare not even read); the exact
authz.MsgExec-wrapping-a-vote shape at 500M (rejected) and 100M(accepted); a governance-lowered cap binding below the default and a raised one admitting 400M; a
zero parameter and a params read failure both falling back to the default; simulation capped too.
test/integration/ante/gasless_gas_limit_test.go(chain-level, real signed txs throughbaseapp → ante → EndBlock, with
max_gasset to-1):TestGaslessCumulativeGasWantedCannotFailFinalizeBlock— the finding itself: two gasless txs eachdeclaring
MaxInt64in one block.FinalizeBlockmust return no error, both txs must berejected, and the block's gas wanted must stay near zero.
TestGaslessTxAboveCapRejected/TestGaslessTxAtCapAccepted— the boundary, asserted throughGetBlockGasWanted(a 100M declaration shows up as >= 50M viaMinGasMultiplier0.5) rather thanthrough the log alone.
TestGaslessTxAtUniversalValidatorGasAccepted— the fleet's new 100,000,000 still votes.TestGaslessCapIsAGovernanceParameter— 40M is admitted under the default, then a realMsgUpdateParamsfrom the gov authority lowers the cap to 30M and the same 40M tx is rejectedon the next block while 30M still passes; a zero cap is refused and leaves the stored cap intact.
x/uexecutor/types/params_test.go— the shipped default, zero rejected, governance-chosen caps accepted.Mutation check
With the enforcement reverted (
if false && gas > maxGas,if false && p.MaxGaslessTxGas == 0) andthe tests kept, the chain-level test reproduces the finding exactly:
and every cap assertion fails with it —
"50000000" is not less than "1000000","20000000" is not less than "1000000",expected: 0x5f5e100 / actual: 0x0, plusShould be falseon each unit-level "must not reach the next decorator". Restored, all three packages are green.
Existing tests updated
x/uexecutor/types/genesis_test.go&GenesisState{}was asserted valid; an emptyParamsnow leaves the cap at 0 and is rejected at genesis. Case flipped tovalid: false.test/integration/uexecutor/evm_hooks_and_outbound_test.goParams{SomeValue: ...}from scratch, dropping the cap. Now mutates a copy of the existing params.test/utils/contracts_setup.goInitGenesis(&GenesisState{})would now fail validation and silently skip the factory deployment. UsesDefaultGenesis().universalClient/pushsigner/vote_test.godefaultGasLimit <= uexecutortypes.DefaultMaxGaslessTxGas, so client and chain cannot drift apart again.Nothing else in-tree declares a gasless gas limit above 100,000,000: the shell scripts use
--gas=auto,50000,300000and600000.Full suite
Companion PR
This is the ante half. The fee-market half — clamping the cumulative gas wanted in
EndBlockinstead of returning an error, and making
AddTransientGasWantedsaturate rather than wrap — ispushchain/push-chain-evm#50. The cap closes the only currently reachable route; the clamp removes
the halt for any future one, independently of
max_gasandMinGasPricestaying sensibly set.Sequence the two together.
Consolidation
One cap retires three findings' recommendations: 18144 rec 6, 18816 rec 3, 18182 rec 6.