fix(evm): reject overflow Amount in CheckTx under ForkEVMFixOverflow - #1306
Closed
vipwzw wants to merge 1 commit into
Closed
fix(evm): reject overflow Amount in CheckTx under ForkEVMFixOverflow#1306vipwzw wants to merge 1 commit into
vipwzw wants to merge 1 commit into
Conversation
EVMContractAction.Amount is uint64 while the underlying account system uses int64. The ForkEVMFixOverflow gate already protects the exec layer (CanTransfer/Transfer/Call/Create), but CheckTx still lets such txs into the mempool and into blocks, where they only fail at execution. Reject Amount > math.MaxInt64 at CheckTx under the same fork, so the fake msg.value attack vector is refused at the mempool/block-check boundary as well. Pre-fork behavior is unchanged. Adds TestCheckTxRejectsAmountOverflow covering: MaxUint64 and MaxInt64+1 rejected post-fork, MaxInt64/1e8/0 accepted post-fork, and pre-fork historical behavior preserved.
Collaborator
Author
|
执行层已经做了拦截(ForkEVMFixOverflow:CanTransfer 的 MaxInt64 守卫、Call/Create 与 transfer-only 路径的 Transfer 返回值检查、token 预编译的 IsInt64 守卫),资金安全已有保障,CheckTx 这个位置不需要额外做拦截。先关闭该 PR。 |
Collaborator
Author
|
testnode 全链路验证补充(master, ForkEVMFixOverflow 默认生效):伪充值交易 Amount=MaxUint64 在执行层被 fork gate 拦截后不会上链(QueryTx → tx not exist),攻击者仅损失部署 gas,evm 执行器账户余额为 0,无资金损失可复现。复现测试:plugin-worktrees/master-evm-check 下 plugin/dapp/evm/executor/fake_deposit_testnode_test.go(TestMasterEvmFakeDepositNoFundLoss, PASS)。 |
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.
Problem
EVMContractAction.Amountisuint64, but the underlying chain33 account system usesint64.Amount > math.MaxInt64wraps to a negative value downstream — the root cause of the fakemsg.valueattack (0-balance fake deposit draining real funds from bank-style contracts).ForkEVMFixOverflow(d4ef8c1) already protects the exec layer (CanTransferrejects overflow,Call/Createrevert onTransferfailure, token precompile guarded, transfer-only path guarded). However,CheckTxstill performs no amount validation (EvmType.Amountalways returns 0, so framework-level checks see nothing), letting such transactions into the mempool and into blocks where they only fail at execution.Fix
Under
ForkEVMFixOverflow,CheckTxnow decodes the action and rejectsAmount > math.MaxInt64withtypes.ErrAmount, so overflow transactions are refused at the mempool / block-check boundary too. Since chain33 runsCheckTxduring block execution as well, this keeps malicious txs out of blocks entirely on fork-active chains.Pre-fork behavior is intentionally unchanged (consensus compatibility).
Tests
New
TestCheckTxRejectsAmountOverflow:MaxUint64andMaxInt64+1rejected withErrAmountMaxInt64,1e8,0acceptedMaxUint64still accepted byCheckTx(historical behavior; exec-layer gate handles it there)go test -ldflags=-checklinkname=0 ./plugin/dapp/evm/executor/— all pass (including the existingTestWBTYOverflowAttackIntegration).