Skip to content

fix(trade): reject order amount overflow and validate price symbol - #1342

Open
vipwzw wants to merge 1 commit into
masterfrom
fix-trade-order-overflow
Open

vipwzw wants to merge 1 commit into
masterfrom
fix-trade-order-overflow

Conversation

@vipwzw

@vipwzw vipwzw commented Oct 19, 2026

Copy link
Copy Markdown
Collaborator

问题

trade 执行器的订单/成交金额全部直接用 int64 乘法,超出 int64 时静默回绕:

  1. tradeSell 的 TotalBoardlot*AmountPerBoardlot、tradeBuyLimit 的 PricePerBoardlot*TotalBoardlot 回绕后只冻结零头;
  2. tradeSellMarket 的成交付款 BoardlotCnt*PricePerBoardlot 同样回绕。攻击者挂一个 PricePerBoardlot=2^32、TotalBoardlot=2^32+1 的买单,只冻结 2^32(约 42.95 枚);卖方卖出全部 boardlot 时付款回绕成 2^32 通过检查,买方却按 AmountPerBoardlot 拿走大量 token;
  3. checkPrice/notSameAsset 调用时把 PriceExec 当成 symbol 传入,导致“资产 == 计价资产”的畸形订单绕过校验。

修复

  • 新增 isMulOverflow,在 tradeSell、tradeBuyLimit、tradeBuy、tradeSellMarket 的挂单冻结、成交付款、token 数量计算处做溢出检测。
  • checkPrice/notSameAsset 分叉后改传 PriceSymbol。
  • 全部分支由 ForkTradeFixOverflow 控制(默认高度 0)。
  • 新增 trade_fix_overflow_test.go 回归用例(溢出卖单/买单拒绝、价符参数校验)。

验证

  • go build ./plugin/dapp/trade/... 通过
  • go test ./plugin/dapp/trade/executor/ -count=1 通过

Two fund-safety defects in the trade executor:

1. Order amounts are multiplied as plain int64. A sell order with
   TotalBoardlot*AmountPerBoardlot wrapping past int64 freezes only the wrapped
   remainder, and a buy-limit order whose PricePerBoardlot*TotalBoardlot wraps
   can later pay the taker seller the wrapped value in tradeSellMarket while
   the buyer receives the full token amount. A malicious buyer can therefore
   take a seller's tokens for a tiny wrapped payment.
2. checkPrice/notSameAsset were called with PriceExec in the symbol argument,
   so the same-asset order check and price-pair validation were ineffective.

- Add isMulOverflow and check every order/payment/token multiplication involved
  in tradeSell, tradeBuyLimit, tradeBuy and tradeSellMarket.
- Pass PriceSymbol into checkPrice/notSameAsset after the new fork.
- Behavior is gated behind ForkTradeFixOverflow (default height 0).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant