fix: cosmwasm evm query path repeatable undercharged evm exec#345
fix: cosmwasm evm query path repeatable undercharged evm exec#345mattkii wants to merge 3 commits into
Conversation
|
Warning Review limit reached
More reviews will be available in 34 minutes and 56 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThe PR updates ERC20-related EVM query handlers to pass a bounded gas cap into internal ABI calls instead of Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Why were these changes required?
| // queryGasCap returns the gas cap to apply to internal EVM query sub-calls. | ||
| // It is bounded by the transaction's remaining SDK gas so that a CosmWasm | ||
| // contract cannot force more EVM computation than its transaction pays for, | ||
| // and additionally capped by DefaultGasCap to match the eth_call behaviour. | ||
| func queryGasCap(ctx sdk.Context) *big.Int { | ||
| return new(big.Int).SetUint64(min(ctx.GasMeter().GasRemaining(), emvconfig.DefaultGasCap)) | ||
| } |
There was a problem hiding this comment.
Interesting approach. Would it not be better to just keep the GasRemaining? I'm saying this because the evm side with the new changes, we kind of already take the minimum
There was a problem hiding this comment.
Check if the suggestion at KiiChain/evm#18 (comment) make sense
2 participants
Description
Fixes a gas-accounting vulnerability in the CosmWasm→EVM query bridge
(
wasmbinding/evm/queries.go). Any CosmWasm contract could call the ERC20information/balance/allowance queries against an arbitrary EVM contract, and each
sub-call was issued with
gasCap=nil. The underlying EVM helper ignores a nil capand falls back to
DefaultGasCap(25M), so an attacker-controlled ERC20 could forcelarge, repeatable internal EVM execution while the SDK gas meter was charged only the
post-refund amount — enabling sustained, undercharged validator compute.
This PR makes every internal ERC20 query sub-call pass a real gas cap bounded by the
transaction's remaining SDK gas (and
DefaultGasCap), mirroring the existingbuildEthCallRequestbehaviour used by the genericEthCallpath.Changes:
queryGasCap(ctx)helper returningmin(ctx.GasMeter().GasRemaining(), DefaultGasCap).queryGasCap(ctx)instead ofnilto all six sub-calls:decimals,name,symbol,totalSupply,balanceOf,allowance.Dependencies: full mitigation also relies on the companion
KiiChain/evmPR(
fix/wasm-repeatable-undercharged-internal-evm-exec) whereCallEVMWithDatanowhonours the gas cap and bills the SDK meter for actual pre-refund EVM work. That fork
version must be published and the
replace github.com/cosmos/evm => github.com/KiiChain/evmdirective in
go.modbumped for the fix to take full effect on-chain.Type of change
How Has This Been Tested?
go test -tags=test ./wasmbinding/evm/...— all query binding tests pass (incl. ERC20 information/balance/allowance and EthCall gas tests).go build ./wasmbinding/...— compiles cleanly.PR Checklist:
make lint-fix(golangci-lint v2.7.2, 0 issues)